Skip to content

💄 [#2064] Fix userfeed styling #1180

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

Merged
merged 2 commits into from
May 8, 2024
Merged

Conversation

jiromaykin
Copy link
Contributor

@jiromaykin jiromaykin commented Apr 25, 2024

issue: https://taiga.maykinmedia.nl/project/open-inwoner/issue/2064

based on design: https://www.figma.com/file/iKGhWhstaLIlFSaND2q7cE/OIP---Designs-(new)?type=design&node-id=1460%3A5026&mode=dev

Fixing heading(=description) styling + improved the pseudo-element spacing a bit ;
not yet adding the arrow link, will be new issue if actually needed in future.

Note: I have also changed the order and HTML semantics, as this relates to this issue: https://taiga.maykinmedia.nl/project/open-inwoner/task/2352

@jiromaykin jiromaykin changed the title [#2064] Decreased userfeed title line-break 💄 [#2064] Fux userfeed styling Apr 25, 2024
@jiromaykin jiromaykin changed the title 💄 [#2064] Fux userfeed styling 💄 [#2064] Fix userfeed styling Apr 25, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.18%. Comparing base (475ae94) to head (0e49907).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1180   +/-   ##
========================================
  Coverage    95.18%   95.18%           
========================================
  Files          956      956           
  Lines        34603    34603           
========================================
  Hits         32937    32937           
  Misses        1666     1666           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jiromaykin jiromaykin force-pushed the fix/2064-userfeed-styling branch 2 times, most recently from 0b2dd55 to 0d66ef4 Compare April 29, 2024 09:47
@jiromaykin jiromaykin force-pushed the fix/2064-userfeed-styling branch 4 times, most recently from 8c6c50b to eaeeafb Compare April 30, 2024 18:01
@jiromaykin jiromaykin force-pushed the fix/2064-userfeed-styling branch from 88e6391 to 0e49907 Compare April 30, 2024 18:06
@jiromaykin jiromaykin marked this pull request as ready for review April 30, 2024 18:09
@jiromaykin jiromaykin requested a review from pi-sigma April 30, 2024 18:12
Copy link
Contributor

@pi-sigma pi-sigma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I noticed two departures from the design:

(1) in the design the the text is bold (ours is regular)
(2) in the design the arrow is always centered in the box (on our end it's aligned with the bottom line of the text)

Double-check that this is as intended, then we're good to go.

@jiromaykin
Copy link
Contributor Author

@pi-sigma Let's look at this together again, because

  1. I don't see a bold/normal difference with the design?
  2. in Figma there is actually no design for a situation where the text exceeds 1 line, and I would say it is important to keep the arrow on the same height or else it would be floating at a different heights. But if we keep the arrow floating at the bottom it looks more peaceful to the eye.:
userfeed-arrow-bottom

@jiromaykin
Copy link
Contributor Author

@pi-sigma I checked again to see if it looks weird, but it doesn't. For the larger cards, like Vragen we also have the arrows 'sticking' to the bottoms of the cards, not the center.
And: The only situation where the 'bold' text would be floating 'above' the arrow, is when it is part of a grid where the card next to it is higher.
It's better to let the card border grow in order to give some peace to the eye in those cases, so it could look like this in a grid where cards that are next to each other will be given the same height:

userfeed-heights

@pi-sigma pi-sigma self-requested a review May 8, 2024 06:45
@alextreme alextreme merged commit 88bd65d into develop May 8, 2024
15 checks passed
@alextreme alextreme deleted the fix/2064-userfeed-styling branch May 8, 2024 11:02
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.

4 participants