Repository navigation
fix(app): open the full nav over the page on compact viewports #3574
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@hyperdx/app': patch | ||
| --- | ||
|
|
||
| fix(app): use the ClickStack hexagon and bolt mark in the logo, the collapsed nav, and the favicon. |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@hyperdx/app': patch | ||
| --- | ||
|
|
||
| fix(app): open the full nav over the page on compact viewports. On narrow screens the sidebar toggle now opens the full nav as an overlay instead of doing nothing. |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,50 +1,25 @@ | ||
| /** | ||
| * ClickStack mark from the hexagon-and-bolt symbol. | ||
| * The bolt uses the page background so it stays two-tone: white on the dark | ||
| * surface, dark on the light one. | ||
| */ | ||
| export default function Logomark({ size = 16 }: { size?: number }) { | ||
| return ( | ||
| <svg | ||
| width={size} | ||
| height={size} | ||
| viewBox="0 0 38.7979 44.7998" | ||
| fill="none" | ||
| aria-hidden | ||
| > | ||
| <path | ||
| fill="currentColor" | ||
| d="M38.7979 11.2002V33.5996L19.3994 44.7998L0 33.5996V11.2002L19.3994 0L38.7979 11.2002Z" | ||
| /> | ||
| <path | ||
| fill="var(--color-bg-body)" | ||
| d="M24.2434 7.39081C24.5455 7.61204 24.6866 8.0663 24.5838 8.48686L21.8763 19.5618H26.8722C27.1721 19.5618 27.4433 19.7862 27.5622 20.1326C27.6812 20.4791 27.6251 20.8814 27.4196 21.1563L15.3924 37.2399C15.1527 37.5603 14.7692 37.6305 14.467 37.4093C14.1649 37.188 14.0237 36.7338 14.1266 36.3132L16.834 25.2383H11.8382C11.5383 25.2383 11.2671 25.0139 11.1481 24.6675C11.0291 24.321 11.0852 23.9187 11.2908 23.6438L23.318 7.56021C23.5576 7.23979 23.9411 7.16958 24.2434 7.39081Z" | ||
| /> | ||
| </svg> | ||
| ); | ||
| } |
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { useCallback, useEffect, useMemo, useRef } from 'react'; | ||
| import Link from 'next/link'; | ||
| import Router, { useRouter } from 'next/router'; | ||
| import Script from 'next/script'; | ||
|
|
@@ -7,7 +7,7 @@ import { Badge } from '@clickhouse/click-ui'; | |
| import HyperDX from '@hyperdx/browser'; | ||
| import { SavedSearchListApiResponse } from '@hyperdx/common-utils/dist/types'; | ||
| import { Anchor, Collapse, Flex, Group, ScrollArea, Text } from '@mantine/core'; | ||
| import { useDisclosure, useFocusTrap, useLocalStorage } from '@mantine/hooks'; | ||
| import { | ||
| IconBell, | ||
| IconChartDots, | ||
|
|
@@ -23,6 +23,7 @@ import api from '@/api'; | |
| import { useLogomark, useWordmark } from '@/brand'; | ||
| import { AlertStatusIcon } from '@/components/AlertStatusIcon'; | ||
| import { InstanceLabel } from '@/components/AppNav/InstanceLabel'; | ||
| import { useMobileNavOverlay } from '@/components/AppNav/useMobileNavOverlay'; | ||
| import { useCompactViewport } from '@/components/CompactViewport/useCompactViewport'; | ||
| import { APP_VERSION, IS_LOCAL_MODE } from '@/config'; | ||
| import { Dashboard, useDashboards } from '@/dashboard'; | ||
|
|
@@ -71,6 +72,9 @@ type NavLinkConfig = { | |
| cloudOnly?: boolean; // Only show when not in local mode | ||
| }; | ||
|
|
||
| const RAIL_WIDTH = 50; | ||
| const FULL_NAV_WIDTH = 250; | ||
|
|
||
| const NAV_LINKS: NavLinkConfig[] = [ | ||
| { | ||
| id: 'chart', | ||
|
|
@@ -182,9 +186,37 @@ export default function AppNav({ fixed = false }: { fixed?: boolean }) { | |
| defaultValue: false, | ||
| }); | ||
|
|
||
| const isCompact = useCompactViewport(); | ||
| const { | ||
| isOpen: isMobileNavOpen, | ||
| toggle: toggleMobileNav, | ||
| close: closeMobileNav, | ||
| } = useMobileNavOverlay(isCompact); | ||
| const collapseButtonRef = useRef<HTMLButtonElement>(null); | ||
| const wasMobileNavOpenRef = useRef(false); | ||
| // Traps Tab inside the open overlay so keys do not reach the page under the scrim. | ||
| const navFocusTrapRef = useFocusTrap(isMobileNavOpen); | ||
|
|
||
| // The toggle stays mounted when the overlay closes, so keyboard focus can | ||
| // return to it after Escape or the scrim removes itself. | ||
| useEffect(() => { | ||
| if (wasMobileNavOpenRef.current && !isMobileNavOpen) { | ||
| collapseButtonRef.current?.focus(); | ||
| } | ||
| wasMobileNavOpenRef.current = isMobileNavOpen; | ||
| }, [isMobileNavOpen]); | ||
|
|
||
| // The page layout reserves the rail width even while the overlay is open, | ||
| // so opening the nav does not shift the content. | ||
| const isRail = isCompact || isPreferCollapsed; | ||
| const isCollapsed = isRail && !isMobileNavOpen; | ||
| const navWidth = isCollapsed ? RAIL_WIDTH : FULL_NAV_WIDTH; | ||
| const reservedWidth = isRail ? RAIL_WIDTH : FULL_NAV_WIDTH; | ||
| // The in-flow rail is border-box, so its 1px border is already inside | ||
| // reservedWidth. The fixed nav's ghost keeps the extra pixel it had | ||
| // before. The overlay ghost must not, or the page shifts when it opens. | ||
| const ghostWidth = | ||
| isMobileNavOpen && !fixed ? reservedWidth : reservedWidth + 1; | ||
|
|
||
| useEffect(() => { | ||
| HyperDX.addAction('user navigated', { | ||
|
|
@@ -321,24 +353,35 @@ export default function AppNav({ fixed = false }: { fixed?: boolean }) { | |
|
|
||
| return ( | ||
| <AppNavContext value={{ isCollapsed, pathname }}> | ||
| {(fixed || isMobileNavOpen) && ( | ||
| <div | ||
| className={styles.navGhost} | ||
| style={{ | ||
| width: ghostWidth, | ||
| minWidth: ghostWidth, | ||
| }} | ||
| ></div> | ||
| )} | ||
| {isMobileNavOpen && ( | ||
|
Contributor
There was a problem hiding this comment. 🔵 minor — The overlay acts like a modal drawer but does not manage focus Opening the nav leaves focus on the toggle, and Tab moves past the nav into page controls hidden behind the scrim. Closing it with Escape or the scrim does not return focus anywhere. Trap focus inside the nav while it is open, for example with Mantine Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond |
||
| <button | ||
| type="button" | ||
| className={styles.navScrim} | ||
| aria-label="Close navigation" | ||
| onClick={closeMobileNav} | ||
| /> | ||
| )} | ||
| <InstallInstructionModal | ||
| show={showInstallInstructions} | ||
| onHide={closeInstallInstructions} | ||
| /> | ||
| <div | ||
| ref={navFocusTrapRef} | ||
| data-testid="app-nav" | ||
| className={cx(styles.nav, { | ||
| [styles.navFixed]: fixed || isMobileNavOpen, | ||
| [styles.navOverlay]: isMobileNavOpen, | ||
| [styles.navCollapsed]: isCollapsed, | ||
| [styles.navCompactRail]: isCompact && !isMobileNavOpen, | ||
| })} | ||
| style={{ width: navWidth }} | ||
| > | ||
|
|
@@ -380,13 +423,20 @@ export default function AppNav({ fixed = false }: { fixed?: boolean }) { | |
| {/* click-ui 0.12 has no sidebar-left icon, so the glyph is | ||
| inlined. Chrome matches IconButton type="primary" size="sm". */} | ||
| <button | ||
| ref={collapseButtonRef} | ||
| type="button" | ||
| className={cx(styles.collapseButton, { | ||
| [styles.collapseButtonCollapsed]: isCollapsed, | ||
| })} | ||
| title="Collapse/expand navigation" | ||
| aria-label="Collapse/expand navigation" | ||
| aria-expanded={!isCollapsed} | ||
| data-autofocus={isMobileNavOpen ? true : undefined} | ||
| onClick={() => | ||
| isCompact | ||
| ? toggleMobileNav() | ||
| : setIsPreferCollapsed((v: boolean) => !v) | ||
| } | ||
| > | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
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.
🟠 major — The nav overlay shares z-index 10 with the row side panel, so the open side panel covers most of the nav
DBRowSidePanel (packages/app/src/components/DBRowSidePanel.tsx:1405, 1461, 1476) renders a portaled Drawer at
contextZIndex + 10= 10, withwithOverlay={false}and a width of 80% (DrawerUtils.tsx:16). The portal comes later in the DOM, so at the same z-index it paints over the in-tree nav. On a 390px viewport with a row open, the toggle at x<50 is still visible and clickable. Clicking it opens a 250px nav whose area from x≈78 to x=250 is hidden under the drawer. The z-9 scrim also sits under the drawer, so the drawer stays undimmed and interactive. Fix: put the overlay above top-level drawers, for example by portaling the nav and scrim with a z-index abovedrawerZIndex, or close open side panels when the nav opens.Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond