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

feat: add nav list to mobile nav #33

Merged
merged 2 commits into from
Nov 8, 2024
Merged

feat: add nav list to mobile nav #33

merged 2 commits into from
Nov 8, 2024

Conversation

limsohee1002
Copy link

Description

add nav list to mobile nac

Type(s) of changes

  • Bug fix
  • New feature
  • Update to an existing feature

Motivation for PR

#32

How Has This Been Tested?

Applicable screenshots

https://www.loom.com/share/6e92539bd5ea456f91e57a2a4c88800c?sid=f93e3e4d-c944-49aa-a13e-dd433331ea7b

Follow-up PR

@limsohee1002
Copy link
Author

There are some confusing UI. The section title and article title has same text size and text color is different. When we add an active state, the color is pink, so it is hard to tell the hierarchy.
I think it would be better to show it to the designer and ask for guidance. Let's handle it on QA? and flag this to designer?

@limsohee1002 limsohee1002 marked this pull request as ready for review November 7, 2024 19:39
@nahbee10
Copy link
Collaborator

nahbee10 commented Nov 7, 2024

There are some confusing UI. The section title and article title has same text size and text color is different. When we add an active state, the color is pink, so it is hard to tell the hierarchy. I think it would be better to show it to the designer and ask for guidance. Let's handle it on QA? and flag this to designer?

Let's proceed and merge it and i'll discuss with Sam next week in the design dev sync with the preview! Thanks for raising the issue.

@nahbee10 nahbee10 merged commit c23e3d9 into master Nov 8, 2024
3 checks passed
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.

2 participants