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

Styles: Cards in group have unequal image heights #949

Open
justintemps opened this issue Apr 17, 2024 · 6 comments
Open

Styles: Cards in group have unequal image heights #949

justintemps opened this issue Apr 17, 2024 · 6 comments
Assignees

Comments

@justintemps
Copy link
Member

Cards in a card group have unequal width depending on their content. Instead, the width of all cards should be the same.

I think this is happening because the Card Group css uses 1fr to allot space whereas it should probably use percentages instead. Example: repeat(4, 25%) instead of repeat(4, 1fr)

See the cards in this example: https://www-preview.ilo.org/es/acerca-de-la-oit

Screenshot 2024-04-17 at 10 04 18 Screenshot 2024-04-17 at 10 04 02 Screenshot 2024-04-17 at 10 03 44
@justintemps
Copy link
Member Author

Hey @Shashika6 thanks for working on this, I think I've phrased the problem incorrectly. The issue that @inesdgomes originally raised and that I misdiagnosed is that the images in the cards are different heights.

In this situation, it's clear that longer words will force cards to be unequal widths. We can't control that, but we should be able to make sure that the heights of the images inside the cards remain the same.

I'm going to close your first PR, because I don't think we should break words just to keep things the same width, that's a very blunt solution.

Instead, what I'd recommend doing is, in the feature cards, add specific flex rules on the ilo--card--image--wrapper and ilo--card--content classes. As you'll see in the below example, I did this and got ok results.

Screenshot 2024-04-17 at 14 29 58

You'll want to carefully consult the Figma designs for this component to get as close as possible to the intended ratio between the images and the content area and then do lots of testing in Storybook to make sure that there aren't any edge cases that this breaks.

Examples:

  1. What happens when there's lots of text?
  2. What happens where the image is portrait instead of landscape?

Let me know what you think.

@justintemps justintemps changed the title Styles: Cards in group have unequal width depending on Content Styles: Cards in group have unequal image heights Apr 17, 2024
@Shashika6
Copy link
Contributor

hey @justintemps, Tried implementing a fix for this with the flex rules that you provided and also tried a few other things but there is at least one case that breaks the card.

Heres how it looks like for flex 0 0 45%

  1. Large screen

Image

  1. Testing responsiveness on large screen with 4 cards.

Image

  1. One card in a large screen.

Image

@justintemps
Copy link
Member Author

@inesdgomes see @Shashika6's comment above. This seems like a complicated problem to fix, for a minor problem that only occurs when you're trying to squeeze four cards into a space where you should really only have three. I think we should close this issue, what do you think?

@justintemps justintemps assigned inesdgomes and unassigned Shashika6 Apr 19, 2024
@GGKapanadze
Copy link
Member

Do we want to achieve something like this?

Screen.Recording.2024-04-19.at.12.47.29.mov
  1. right now, we don't have a min-width for those cards, and that's the main issue: the screen implementation you are looking at utilizes minmax
grid-template-columns: repeat(auto-fit, minmax(150px, 1fr));
  1. we are trying to pass amount of card as a class to style them correctly
    image
    image
    But if we want to use a grid we can do fit-content and utilize as much space as the parent gives us, or we can use flexbox with min-width too

@justintemps
Copy link
Member Author

Thanks @GGKapanadze, the count classname determines how many cards should fit into each row, assuming that there might be more than one. So may be you have lots of space, but you still only want two cards per row for some reason.

The problem here is just that the image in the card changes height if one of the cards has to be wider than the others, because of some long word in the title.

@beatrizmartinmartins
Copy link
Collaborator

Hey team, I don't know if this could help or not but have we tried working with these breakpoints we created a while ago? This would solve the problem when we resize our screen because it should automatically move to a more "tablet" look https://www.figma.com/file/TnVsWy6VobbHzZdbHdx3Rb/ILO-Page-Templates?type=design&node-id=3980%3A44686&mode=design&t=cjmAwAxdGif8pRGI-1 @justintemps @GGKapanadze

@justintemps justintemps removed the 2024:6 Release by 30 April 2024 label Apr 30, 2024
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 a pull request may close this issue.

5 participants