Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lab images #1064

Merged
merged 15 commits into from
Jan 23, 2023
Merged

Lab images #1064

merged 15 commits into from
Jan 23, 2023

Conversation

okaycj
Copy link
Contributor

@okaycj okaycj commented Jan 20, 2023

This PR adds the ability for labs to add a banner and badge image to their customized lab page. The images are validated. The badge image must be square and the banner ratio must be above 2.

Additionally, the lab's description field's help text was updated.

Closes #918

Screenshot 2023-01-20 at 4 27 21 PM

Screenshot 2023-01-20 at 1 29 52 PM

Screenshot 2023-01-20 at 1 30 04 PM

Screenshot 2023-01-20 at 1 30 15 PM

@okaycj okaycj changed the base branch from develop to study-priority January 20, 2023 18:32
@okaycj okaycj changed the base branch from study-priority to develop January 20, 2023 19:45
@okaycj okaycj marked this pull request as ready for review January 20, 2023 19:57
@mekline
Copy link
Contributor

mekline commented Jan 20, 2023

Thanks! Can you please make new issues for the "TODO" steps if you're not completing them this sprint?

Or if you have time, here is a revision for these fields:

Banner Image: This image will be shown at the top of your custom URL page, when it is viewed on a laptop/wide browser window. Please keep your file size less than 1 MB.

Badge Image: This image will be shown at the top of your custom URL page when it is viewed on a mobile device/narrow browser window, and as a badge/avatar image for your lab. This image should be square. Please keep your file size less than 1 MB.

And here are a couple more things, ideal to implement now if time, otherwise please make new issues:

  • Should we validate the shapes, as with the study images? For the banner, we could specify a range like between 1:7 and 1:3.
  • To make thing flow more smoothly it would be great if you could move these two items to immediately after the custom-url box.
  • Amendment to the help text for the "Description" field: "A short (2-3 sentences), parent-facing description of what your lab studies or other information of interest to families. This description will be shown under the banner image on your custom URL page."

@okaycj
Copy link
Contributor Author

okaycj commented Jan 23, 2023

@mekline

Thank you for the giving me to copy. I added it to this PR.

  • Should we validate the shapes, as with the study images? For the banner, we could specify a range like between 1:7 and 1:3.

The new image fields are validated. The badge image needs to be square and the banner image is validated to be above a ratio of 2 (w:h). Of course, let me know if you'd like different behavior.

  • To make thing flow more smoothly it would be great if you could move these two items to immediately after the custom-url box.
  • Amendment to the help text for the "Description" field: "A short (2-3 sentences), parent-facing description of what your lab studies or other information of interest to families. This description will be shown under the banner image on your custom URL page."

I added commits for these two points. Thank you for the feedback.

@sonarcloud
Copy link

sonarcloud bot commented Jan 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mekline
Copy link
Contributor

mekline commented Jan 23, 2023

Okay, that all sounds good!!

@okaycj okaycj merged commit 5fdc9ef into develop Jan 23, 2023
@okaycj okaycj deleted the lab-images branch January 23, 2023 17:01
@okaycj okaycj mentioned this pull request Jan 23, 2023
@becky-gilbert
Copy link
Contributor

becky-gilbert commented Jan 25, 2023

@okaycj @mekline I know this has been merged but I have a few questions about things I noticed while updating the documentation.

I added banner and badge images to the demo lab on staging in order to get some screenshots, and I noticed two things:

  1. When the banner image is shown, I don't see the lab name on the page. E.g. "Sandbox Lab" is shown on the 3rd and 4th screenshots from CJ's post above but not on the 2nd. Is that intentional? If not, let me know and I can try to add it.
  2. Based on the help text in Melissa's comment, I expected the lab page to show the banner image on wide browser windows and badge image on narrow windows. But when the lab has both image types, I only see the banner image. I don't see the badge image on the lab page, even when I navigate to the page from a narrow window or emulate mobile (screenshot below). I do see the badge image on the lab page after I've cleared the lab's banner image.

WRT to question 2 - if it would be helpful, I can either (a) try to implement the functionality that I was expecting, i.e. banner on wide browser and badge on narrow/mobile (but might take a while to figure out), or (b) edit the form's help text to reflect the current behavior, i.e. the badge image will be shown on the lab page if there is no banner image (faster).

Here's a screenshot of the demo lab page on staging, from a desktop browser using the mobile emulator. Note that this page shows the banner rather than badge image, and there's no 'Demo Lab' title.

staging_lab_page_mobile

@becky-gilbert
Copy link
Contributor

Answers to my questions after talking to Melissa and CJ:

  1. This is intentional. The idea is that the lab will probably have a banner image that includes their lab name. I'll add a note in the docs about this. (At some point in the future we may decide to add the lab name to the page text as well.)
  2. The images should switch based on browser window dimensions but we're not going to implement that in this shape - it'll be part of our mobile compatibility/responsiveness work. I'll make a note in the docs about how this works now, and we can change it later once we get the image types to display as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Customized header images and text for lab pages
3 participants