-
Notifications
You must be signed in to change notification settings - Fork 1
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
[UI] Add Quests and Developer links for Store consumption #431
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. might need to update the breakpoints in the consuming code.
Also would be good if we could refactor this component to be more general. i.e. take the url's as props and the link text as props as well so we can support translations. We could make it take a list of nav item components so we wouldn't need to send ui lib pr's for small stuff like this
@BrettCleary I figured this would need some refactoring as a follow up - esp to be able to use with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, the only change that would be a plus is what Brett mentioned, updating breakpoints, also taking in consideration the distance between the menu and the logo for small screen (give it more breath space).
Screen.Recording.2025-02-12.at.10.14.41.AM.mov
@BrettCleary @biliesilva fixed some responsiveness at a weird point between tablet and desktop, swapped old twitter logo