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: [M3-7366] - Improved Primary Navigation UX #10137

Merged
merged 20 commits into from
Mar 28, 2024

Conversation

jaalah-akamai
Copy link
Contributor

@jaalah-akamai jaalah-akamai commented Feb 1, 2024

Description 📝

Overall improvements to primary nav, which includes changes to accommodate longer Cloud Manager feature names.

Changes 🔄

  • Increased nav width to 232px
  • Added transition to nav links and adjusted logo transition to match
  • Tightened spacing around logos, links, and beta chip according to mocks
  • Reduced link font size to .875rem/14px

Screenshot

Before After
Screenshot 2024-02-07 at 12 35 54 PM Screenshot 2024-02-07 at 12 37 02 PM

How to test 🧪

  • Checkout branch or preview link
  • Verify that opening / closing menu works as intended
  • Verify that changing breakpoints works as intended
  • Verify changes in different browsers

@jaalah-akamai jaalah-akamai self-assigned this Feb 1, 2024
@jaalah-akamai jaalah-akamai requested a review from a team as a code owner February 1, 2024 16:53
@jaalah-akamai jaalah-akamai requested review from jdamore-linode and mjac0bs and removed request for a team February 1, 2024 16:53
@jaalah-akamai jaalah-akamai changed the title Prototype: Navigation Prototype: Navigation [Do Not Merge] Feb 1, 2024
Copy link

github-actions bot commented Feb 1, 2024

Coverage Report:
Base Coverage: 81.69%
Current Coverage: 81.7%

@jaalah-akamai jaalah-akamai changed the title Prototype: Navigation [Do Not Merge] feat: [M3-7745] - Improved Primary Navigation UX Feb 7, 2024
@jaalah-akamai jaalah-akamai marked this pull request as draft February 7, 2024 18:05
@jaalah-akamai jaalah-akamai changed the title feat: [M3-7745] - Improved Primary Navigation UX feat: [M3-7366] - Improved Primary Navigation UX Feb 7, 2024
Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

@jaalah-akamai heads up i rebased the PR

I think we still need to handle the vertical centering of both the logo and the managed menu item?

Screenshot 2024-03-13 at 10 20 22

packages/manager/src/MainContent.tsx Outdated Show resolved Hide resolved
@jaalah-akamai
Copy link
Contributor Author

Managed Default
Screenshot 2024-03-14 at 1 16 37 PM Screenshot 2024-03-14 at 1 18 09 PM

Copy link
Contributor

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Nice! Looks slightly better imo!

I do feel like the green accent colors need to go 🗑️ Is this something we'd consider doing now?

For example, blue looks slightly more appealing to me
Screenshot 2024-03-14 at 1 43 24 PM

@abailly-akamai
Copy link
Contributor

@bnussman-akamai that's a great suggestion - let's go blue! @jaalah-akamai?

@mjac0bs
Copy link
Contributor

mjac0bs commented Mar 14, 2024

@bnussman-akamai that's a great suggestion - let's go blue! @jaalah-akamai?

goblue

As an alum, I couldn't resist this one opportunity. 😆

@jaalah-akamai
Copy link
Contributor Author

@bnussman-akamai @abailly-akamai @mjac0bs UX wants to hold on the color change especially since we'll be introducing design tokens soon and the colors will shift slightly. It's on our radar though!

@jaalah-akamai
Copy link
Contributor Author

This is good to go otherwise

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

This looks great on Chrome, Safari, and Firefox at various breakpoints. 🚀

The only thought I had (and perhaps only because I was sitting there clicking this button repeatedly), is that the fade out of the text feels a little slow when collapsing the menu.

Screen.Recording.2024-03-27.at.9.29.04.AM.mov

"@linode/manager": Tech Stories
---

Improved Primary Navigation UX ([#10137](https://github.com/linode/manager/pull/10137))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Improved Primary Navigation UX ([#10137](https://github.com/linode/manager/pull/10137))
Improve Primary Navigation UX ([#10137](https://github.com/linode/manager/pull/10137))

Nit: present tense

Copy link
Contributor

@hana-linode hana-linode left a comment

Choose a reason for hiding this comment

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

I feel like the divider spacing between the logo and the items are a bit cramped in comparison to the other dividers in the nav. Can we increase the spacing? LGTM otherwise!

image

@hana-linode hana-linode added the Needs extra approvals Use for PRs that have high-impact changes where > 2 approvals are needed label Mar 27, 2024
@jaalah-akamai
Copy link
Contributor Author

I feel like the divider spacing between the logo and the items are a bit cramped in comparison to the other dividers in the nav. Can we increase the spacing? LGTM otherwise!

That's by design so that the border lines up with the top menu

@jaalah-akamai jaalah-akamai merged commit b0e50de into linode:develop Mar 28, 2024
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs extra approvals Use for PRs that have high-impact changes where > 2 approvals are needed Ready for Review UX/UI Changes for UI/UX to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants