-
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
DM-48644: Create a PrimaryNavigation component #179
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 88a9654 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
94f912e
to
7856e72
Compare
jonathansick
added a commit
to lsst-sqre/phalanx
that referenced
this pull request
Feb 7, 2025
This component wraps Radix UI's NavigationMenu and provide's a replacement for the current header navigation in Squareone. With NavigationMenu we'll be able to give individual menu items submenus, and not necessarily have to treat the user menu as a special case. This work styles the basic PrimaryNavigation component and demonstrates it with a storybook story. More work is need to make the submenu appear directly below its trigger.
This is a better and more consistent user experience than a menu item opening on hover.
This makes types available for DOM APIs, which we do use in react libraries like Squared.
The Radix navigation menu currently has a flaw where the content viewport is always centered under the navigation menu as a whole, rather than under the trigger specifically. This isn't suitable for Squareone because our navigation menu spans the entire window so the menu will appear disassociated with the trigger. This solution is from the GitHub issue thread and uses a MutationObserver to update the position of the viewport. radix-ui/primitives#1462 (comment) It has the side-effect right now of animating the menu sliding from the centre to the trigger on first view. Perhaps we can tweak the animations so this doesn't happen? Also, we might need to add code to updatePosition that takes into account the browser viewport to ensure we don't push menu content outside the view.
This makes these utility functions more accessible as we redesign the Gafaelfawr user menu to use the PrimaryNav component.
Ensures that the menu appears above content later in the page.
Now we test whether the menu trigger is near the right side of the menu. If it is, we switch to using the "right" for absolute alignment. If it is not, we use the "left". In both cases, we use the absolute alignment rather than doing transformX to avoid having the menu slide sideways, which doesn't make sense since the menu now opens on click rather than hover.
This prevents the footer from shrinking by flexbox. We didn't see any issnegative issues with this property being unset, though.
First steps to adopting the PrimaryNav into the Header. There's a weird bug where the Login component is being rendered server-side, and hence the Logged-in state isn't always available. This replaces the existing HeaderNav, which composed basic Next links in a flex nav along with a GafaelfawrUserMenu with the PrimaryNav where the User menu is now a regular menu item.
- enableAppsMenu is a feature flag - appLinks is an array of menu items Add appLinks configurations This configuration pairs with the enableAppsMenu and sets the content of the Apps menu.
This is an initial implementation that demonstrates an Apps menu that's displayed when the enableAppsMenu setting is added, and shows the contents of the appLinks configuration. This Apps menu implementation is challenging because it doesn't seem to work well with the Next Link component, even if I use the asChild composition method. I've tried to work around this by using a link component that uses a span with a router.push onClick handler, but then this breaks keyboard navigation. More work is needed.
Use the basic PrimaryNavigation.Link for external links, which becomes an <a> tag. For internal links, use the special span with an onClick handler to act like the Nextjs Link component, but working around the issue of having onClick handlers on that Link component from Radix Navigation Menu.
This sets the right cursor style for menu triggers. Previously it was just the default arrow.
We have to set this manually since we're implementing Next links with an onClick handler to push the router to workaround a Next Link + NavigationMenu.Link conflict.
694f628
to
88a9654
Compare
jonathansick
added a commit
to lsst-sqre/phalanx
that referenced
this pull request
Feb 12, 2025
Merged
jonathansick
added a commit
to lsst-sqre/phalanx
that referenced
this pull request
Feb 19, 2025
This version provides the new configurable Apps menu. lsst-sqre/squareone#179
aibsen
pushed a commit
to lsst-sqre/phalanx
that referenced
this pull request
Feb 24, 2025
This version provides the new configurable Apps menu. lsst-sqre/squareone#179
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PrimaryNavigation component in Squared wraps Radix UI's NavigationMenu and provide's a replacement for the current header navigation in Squareone.
Using this new PrimaryNavigation component, this PR also adds an Apps menu item to the header navigation in Squareone that's configurable per environment.