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

Item: improve title and info wrapping #6447

Merged
merged 3 commits into from
Aug 16, 2024

Conversation

distantnative
Copy link
Member

@distantnative distantnative commented May 12, 2024

This PR …

Enhancements

  • Sections: improved title and info wrapping

For review team

  • Add lab and/or sandbox examples (wherever helpful)
  • Add changes & docs to release notes draft in Notion

@distantnative distantnative added the type: enhancement ✨ Suggests an enhancement; improves Kirby label May 12, 2024
@distantnative distantnative added this to the 4.3.0 milestone May 12, 2024
@distantnative distantnative requested a review from a team May 12, 2024 12:39
@distantnative distantnative self-assigned this May 12, 2024
@bastianallgeier
Copy link
Member

Just a few thoughts:

  • How does it look like when the image is not cropped?
  • How does it look if one item in the list is wrapping, but the others are not? Is it weird if the height is different?
  • The info text does not seem to wrap in the cardlets component

@distantnative
Copy link
Member Author

distantnative commented May 17, 2024

How does it look like when the image is not cropped?

I think fine.

Screenshot 2024-05-17 at 16 52 55 Screenshot 2024-05-17 at 16 53 28

How does it look if one item in the list is wrapping, but the others are not? Is it weird if the height is different?

Not super pretty, but I don't think there is a way that looks super pretty and still allows the same utility of being able to read all the information for long titles/info. I think it's a very good compromise that looks ok even in those extreme cases.

Screenshot 2024-05-17 at 16 54 57

The info text does not seem to wrap in the cardlets component

Those are still like before. The title wraps, the info not. If you think it should here as well, we can change that, sure.

@bastianallgeier
Copy link
Member

Uhhh, I just thought that we could maybe use grid-auto-row for the list items. I'm not entirely sure if it really works, but normally it takes care of always setting all items to the same height. That could clean up the list quite nicely.

The images look great indeed.

@distantnative
Copy link
Member Author

@bastianallgeier I looked into grid-auto-rows but I don't understand (or could make it happen when trying) how this should work.

@distantnative distantnative modified the milestones: 4.4.0, 4.5.0 Aug 13, 2024
@bastianallgeier bastianallgeier changed the base branch from develop-minor to v5/develop August 16, 2024 07:16
@bastianallgeier bastianallgeier modified the milestones: 4.5.0, 5.0.0-alpha.3 Aug 16, 2024
@bastianallgeier bastianallgeier merged commit af58732 into v5/develop Aug 16, 2024
13 checks passed
@bastianallgeier bastianallgeier deleted the lab/nico/item-title-wrapping branch August 16, 2024 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement ✨ Suggests an enhancement; improves Kirby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants