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.

Sign up for GitHub

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

Jump to bottom

🥔✨ SectionNavigation: Include Section#description in SectionLink #2057

Merged
merged 9 commits into from
Dec 28, 2023
Merged

🥔✨ SectionNavigation: Include Section#description in SectionLink #2057

merged 9 commits into from
Dec 28, 2023

Conversation

zspencer
Copy link
Member

@zspencer zspencer commented Dec 21, 2023 •
edited
Loading

TODO

  • Make Pretty
  • Fix tests?
  • Split the CardComponent changes out?

After

Screenshot 2023-12-26 at 8 39 41 PM Screenshot 2023-12-26 at 8 39 34 PM

@zspencer zspencer force-pushed the section-navigation/include-description branch 2 times, most recently from 962618d to 0ee62f4 Compare December 27, 2023 03:36
@zspencer zspencer force-pushed the section-navigation/include-description branch 2 times, most recently from e2c4822 to 9f2559f Compare December 27, 2023 05:33
@zspencer zspencer marked this pull request as ready for review December 27, 2023 05:35
@zspencer zspencer changed the title SectionNavigation: Include Section#description in SectionLink 🥔✨ SectionNavigation: Include Section#description in SectionLink Dec 28, 2023
@zspencer zspencer requested review from a team December 28, 2023 01:17
anaulin
anaulin approved these changes Dec 28, 2023
View reviewed changes
Copy link
Member

@anaulin anaulin 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, thank you!

I would like to keep some kind of hover action, or at least the arrow icon, just to make sure that the cards still look/feel like a navigation component.

@zspencer zspencer force-pushed the section-navigation/include-description branch from 81c7364 to 34c5335 Compare December 28, 2023 02:04
@zspencer
Copy link
Member Author

Screenshot 2023-12-27 at 6 23 41 PM

OK this is where I landed; it's not perfect (I can't get the arrow to align well with the words ffs) but it now has a hover state to make it more obvious that it's button; and it has an arrow which should also make it more visually buttony.

@zspencer zspencer merged commit 12063b0 into main Dec 28, 2023
4 checks passed
@zspencer zspencer deleted the section-navigation/include-description branch December 28, 2023 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anaulin anaulin anaulin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zspencer @anaulin