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

Propose specs for images #76

Closed
gissoo opened this issue Feb 6, 2023 · 10 comments
Closed

Propose specs for images #76

gissoo opened this issue Feb 6, 2023 · 10 comments
Assignees
Labels
🗺️ design Tracks design work in an external app

Comments

@gissoo
Copy link
Contributor

gissoo commented Feb 6, 2023

What's in scope for this issue:

  • min and max width
  • max height
@gissoo gissoo added the 🗺️ design Tracks design work in an external app label Feb 6, 2023
@gissoo gissoo self-assigned this Feb 6, 2023
@gissoo gissoo added the 💬 awaiting review Ready for comments and questions label Feb 8, 2023
@jhimpele
Copy link
Contributor

jhimpele commented Feb 8, 2023

sorry, what image or object does this refer to?

@gissoo
Copy link
Contributor Author

gissoo commented Feb 8, 2023

@rlskoeser @jhimpele please review these minimum and maximum widths I've proposed for portrait and landscape images on desktop and mobile, and let me know your thoughts and if I've missed something.

Note:

  • portrait images tend to get too large, so I've tried to make the max width smaller than the full width of the text in the panel
  • landscape images tend to be get hard to read so I've tried to keep the minimum width as large as possible – let me know if you think they're still small I can increase the width.
  • I'm proposing a max height of 450px for portrait images on desktop and mobile – they'll become hard to read if they're taller than that.

@jhimpele
Copy link
Contributor

jhimpele commented Feb 9, 2023 via email

@rlskoeser
Copy link
Contributor

@gissoo the sizes you have in Figma look reasonable to me. I'm not sure that the exact pixels you've noted are what we should go by, though (more on that in one moment). I will configure the image sizes based on margins and max width and height.

If there is a conflict between portrait min width and max height, how do we resolve?

I agree that large landscape images may not be very effective in the panel, but maybe we can handle that by having the team crop them if needed. Do you want to suggest any aspect ratio constraints?

For portrait, max-height is more important to me than width. I don't see that noted in Figma anywhere, but you have it documented in this issue, so as long as we get it documented into the development issue, that's fine.

I was just inspecting on the dev site, it looks like the widest the panel can ever be in the current layout is 768px (largest screen size before we switch from mobile to desktop view). So image sizes should be based on that size panel with the margins you're proposing, and then we need them at least at 2x resolution for retina displays.

Does this give you enough information to determine needed image resolution, or is that something I should do?

@gissoo gissoo removed the 💬 awaiting review Ready for comments and questions label Feb 22, 2023
@gissoo
Copy link
Contributor Author

gissoo commented Feb 27, 2023

@gissoo the sizes you have in Figma look reasonable to me. I'm not sure that the exact pixels you've noted are what we should go by, though (more on that in one moment). I will configure the image sizes based on margins and max width and height.

  • @rlskoeser I made a few revisions to the dimensions – I'm calculating them differently now based on the panel's margins for the max/min width reqs. I have marked the new margins. The ones with drastic change are max width for portrait and max and min width for landscape.

If there is a conflict between portrait min width and max height, how do we resolve?

  • I think we should prioritize the max height.

I agree that large landscape images may not be very effective in the panel, but maybe we can handle that by having the team crop them if needed. Do you want to suggest any aspect ratio constraints?

  • I don't think aspect ratios would be helpful, but as you mentioned it is helpful for cropping portait images. So I've added a note and made a recommendation here on Figma

For portrait, max-height is more important to me than width. I don't see that noted in Figma anywhere, but you have it documented in this issue, so as long as we get it documented into the development issue, that's fine.

  • I have added it to the figma file.

I was just inspecting on the dev site, it looks like the widest the panel can ever be in the current layout is 768px (largest screen size before we switch from mobile to desktop view). So image sizes should be based on that size panel with the margins you're proposing, and then we need them at least at 2x resolution for retina displays.

  • I have added specs for tablet in the figma file, thanks for bringing this up. Let me know if it makes sense.

Does this give you enough information to determine needed image resolution, or is that something I should do?

it might be worth to talk through these together.

@gissoo gissoo added the 💬 awaiting review Ready for comments and questions label Feb 27, 2023
@jhimpele
Copy link
Contributor

jhimpele commented Feb 28, 2023 via email

@gissoo
Copy link
Contributor Author

gissoo commented Mar 1, 2023

@jhimpele Thanks for asking! We'll definitely scroll within the panel to view portrait images, that is inevitable. The reason for recommending a max height for portrait images is to avoid using images that are too long, which would make them hard and awkward to view, and are often not legible on mobile. This might happen because of how an image had previously been cropped. Rebecca might have more to say about this.

@jhimpele
Copy link
Contributor

jhimpele commented Mar 2, 2023

@gissoo Got it. Thanks!

@rlskoeser
Copy link
Contributor

@gissoo thanks for documenting max heights and adding tablet — that will be important to make sure we have the best minimum resolution calculated properly.

Do you need anything further from me to close this issue?

@gissoo
Copy link
Contributor Author

gissoo commented Mar 6, 2023

@rlskoeser I don't think I need anything! When you get to implementation if you notice anything that doesn't make sense to you let's resolve in real time, I anticipate some gaps in our approaches with this.

@gissoo gissoo closed this as completed Mar 6, 2023
@gissoo gissoo removed the 💬 awaiting review Ready for comments and questions label Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗺️ design Tracks design work in an external app
Projects
None yet
Development

No branches or pull requests

3 participants