DM-52394: Implement layout for settings screens (SidebarLayout, SettingsLayout) #208
Conversation
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
🦋 Changeset detectedLatest commit: e723f9c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
jonathansick
force-pushed
the
tickets/DM-52394
branch
from
3b9bece to
2fd7958
Compare
Create the base SidebarLayout component with TypeScript types for props including children, sidebarTitle, and navSections. Includes basic Storybook story and unit tests for initial rendering.
Implement desktop layout with 18rem sidebar, content area, and 2rem gap using CSS Grid. Centers layout with 60rem max width constraint. Includes stories demonstrating layout with various content sizes.
Implement mobile layout with vertical stacking and proper padding below 60rem viewport. Navigation area prepared for disclosure pattern. Includes mobile viewport stories and responsive tests.
- Add @storybook/addon-essentials to enable viewport functionality - Import INITIAL_VIEWPORTS from 'storybook/viewport' - Set default viewport to 'responsive' for better developer experience - Fix missing mdxDir property in mock app config - Update mobile stories to use 'iphone14' viewport for consistent sizing
- Add Sidebar component that renders title and navigation sections with optional labels - Uses semantic HTML structure with proper heading hierarchy - Includes comprehensive Storybook stories with play functions for testing - Integrates with SidebarLayout component with titleHref resolution logic
Implement sticky sidebar with independent scrolling when content exceeds viewport height. Desktop sidebar stays fixed while main content scrolls. Includes stories showing scroll behavior.
Create accessible hamburger menu button using FontAwesome icon. Includes ARIA attributes and responsive visibility. Only shown on mobile viewports. Includes a11y tests and state stories.
Implement mobile header with sidebar title and hamburger button using flexbox layout. Header sticks to top of viewport on mobile. Includes stories showing header states and layout tests.
Add accessible disclosure pattern for mobile navigation menu using react-a11y-disclosure. Menu slides down/up with smooth animation. Includes stories with interaction tests demonstrating disclosure behavior and navigation menu closing on item selection.
Implement keyboard navigation with proper tab order and Escape key handling for mobile menu. Add skip link for accessibility. Focus management ensures proper flow. Includes keyboard interaction tests.
Implement visual feedback for navigation with hover backgrounds and active page indicators. Active items show bold text and left border. Includes stories demonstrating all visual states.
Set up comprehensive unit testing with vitest alongside Storybook interaction tests. Unit tests focus on component logic, props validation, and edge cases. Provides dual testing approach.
Add onNavigate prop to SidebarLayout to prevent navigation during Storybook interaction tests. Remove problematic play functions that triggered browser navigation to non-existent pages, causing websocket connection failures. Maintains story functionality while enabling reliable test execution.
Update SidebarNavItem to accept optional event parameter in onNavigate prop and add preventDefault() handlers in ClickInteraction and InteractionTest stories. This prevents navigation to non-existent pages during Storybook testing, eliminating the remaining websocket connection failures. All 54 Storybook tests now pass successfully.
- Added keyboard navigation support to SidebarNavItem component with Enter and Space key handling - Fixed preventDefault calls in all onNavigate handlers to prevent JSDOM navigation errors - Updated component type signatures to accept both MouseEvent and KeyboardEvent parameters - Fixed test selectors to handle multiple 'Settings' elements and aria-hidden sidebar containers - Updated MobileMenuToggle keyboard accessibility test to use proper button click simulation All 107 unit tests now pass successfully.
- Change skip link background from primary-600 to primary-700 to meet WCAG AA color contrast requirements (4.13:1 → ~7.2:1) - Add CSS visibility:hidden for focusable elements when sidebar has aria-hidden="true" to resolve aria-hidden-focus violation - Set a negative tabindex for hidden items - Ensure desktop sidebar remains accessible with visibility:visible override - Add transition for visibility changes to maintain smooth animations Resolves both serious accessibility violations found in Storybook a11y scan.
- Change MobileHeaderTitle from h1 to h2 for consistent heading levels - Hide SidebarTitle on mobile to prevent duplicate content display - Add aria-label to SidebarRoot for screen reader landmark identification - Ensures single title per context while maintaining accessibility Mobile now shows only header title (h2), desktop shows only sidebar title (h2), eliminating semantic confusion and content duplication.
- Replace border-left with ::before pseudo-element for active indicator - Use separate border-radius controls for hover vs active states - Add 2px rounded active bar that's independent of main element - Modify active state border-radius to blend with indicator bar - Remove padding adjustment since no border affects layout Creates cleaner separation between rounded hover background and straight active indicator, with seamless visual integration when both states are present.
- Add negative left margin to NavigationLink (-0.75rem) to extend interactive area to container edge while maintaining text padding - Add matching left padding to SidebarRoot (0.75rem) to provide space for negative margin and prevent clipping of active state bar - Ensures perfect alignment between navigation text and section headers - Active state bar now extends to true container edge for clean design Navigation items maintain proper text spacing while aligning with "Settings" title and "SECURITY" section labels.
- Change padding from 1rem var(--size-screen-padding-min) to 0.5rem 0 - Reduces vertical padding from 1rem to 0.5rem for tighter header - Removes horizontal padding for edge-to-edge layout on mobile - Creates more space for content while maintaining touch targets Results in a less prominent mobile header that gives more screen real estate to the main content area.
- Eliminate spacing between nav items (margin-bottom: 0.25rem → 0) - Reduce section-to-section spacing by half (margin-bottom: 2rem → 1rem) - Reduce section label to items spacing by half (0.5rem → 0.25rem) - Reduce nav item vertical padding by half (0.5rem → 0.25rem) Creates a more space-efficient sidebar navigation while maintaining readability and adequate touch targets for mobile interaction.
- Add viewport tracking with useState and resize listener - Conditionally apply disclosure contentProps only on mobile viewports - Prevent aria-hidden="true" from being applied to sidebar on desktop - Maintain mobile disclosure functionality while fixing desktop a11y - Match CSS breakpoint: ContentMaxWidth = 60rem = 960px Resolves accessibility violation where sidebar had aria-hidden="true" but contained focusable navigation links on desktop viewports.
Create SettingsLayout component that wraps SidebarLayout with settings-specific navigation configuration. Includes static navigation items for Account, Access Tokens, and Sessions pages. Exports getLayout function for Next.js page integration.
Create comprehensive stories demonstrating SettingsLayout in various states including different active pages and mobile responsive behavior. Stories use AppConfigDecorator for proper context setup and mock Next.js router for navigation testing.
Create Account, Access Tokens, and Sessions pages with proper getServerSideProps implementation for appConfig loading. Pages use SettingsLayout via getLayout pattern and include placeholder content for future development.
Add configuration option to control visibility of Sessions page in settings navigation. Defaults to true for backward compatibility. Allows deployment-specific control over which settings sections are available to users.
- Update SettingsLayout to use useAppConfig hook for dynamic config access - Add useMemo optimization for navigation computation performance - Update getSettingsNavigation to filter based on settingsSessionsVisible config - Sessions section now conditionally appears based on configuration - Maintain backward compatibility with default showing Sessions - Tested with both settingsSessionsVisible: true/false configurations
- Import and use useGafaelfawrUser hook from @lsst-sqre/squared
- Display dynamic title: "{username} Settings" when user is available
- Fallback to "Settings" when username is unavailable
- Use optional chaining for safe access to user.username
- Handle loading state gracefully without loading indicators
- Tested with user "vera" showing "vera Settings" in sidebar title
- Add SessionsHidden story to test navigation filtering with settingsSessionsVisible: false - Add SessionsVisible story to test navigation filtering with settingsSessionsVisible: true - Create mock configurations to test different dynamic scenarios - Use AppConfigProvider decorators to override default Storybook config - Stories demonstrate Sessions section showing/hiding based on configuration - Provides visual testing for dynamic navigation filtering implementation
Create comprehensive unit tests for settingsNavigation function covering dynamic filtering based on configuration. Tests verify: - Navigation generation with Sessions visible/hidden - Default behavior when settingsSessionsVisible is undefined - Structure validation of navigation items and sections - Edge case handling and pure function behavior - Side effect isolation for unrelated config properties
Update titleHref tests to handle multiple "Settings" links (mobile header and sidebar title) by using getAllByRole instead of getByRole and verifying both links have the expected href attribute.
Replace aria-label selector with data-testid for mobile menu toggle button
in MobileMenuToggle story. The getByRole with name pattern was failing to
match the dynamic "Open navigation menu" / "Close navigation menu" aria-label.
Using getByTestId('mobile-menu-toggle') provides more reliable element selection.
Getting the username and setting it in the title was causing a SSR hydration issue. We can work around this most likely, but this is the expedient solution to get the feature off the ground.
Remove client-side mobile viewport detection and conditional props spreading that caused SSR/CSR mismatches in SidebarLayout. Replace with explicit prop handling to ensure consistent rendering across server and client. This resolves React hydration error #418 in production builds.
Resolve ARIA attribute issues introduced by previous hydration fixes: - Add id prop support to MobileMenuToggle component to fix aria-labelledby reference error - Replace prop spreading with explicit prop handling to maintain proper disclosure relationships This eliminates accessibility violations while preserving SSR hydration compatibility. Storybook a11y tests now pass with 26 checks passing (up from 24) and 0 inconclusive violations.
Consolidates the Login component to fix hydration mismatches by removing the complex next/dynamic + LoginClient architecture. The component now renders the "Log in" link consistently on both server and client, transitioning to UserMenu only after hydration and authentication data are available. Changes: - Add useState/useEffect pattern to track mount state - Always render empty fragment during SSR and initial client render - Only render user menu after client hydration completes - Merge LoginClient logic directly into Login component - Remove LoginClient.tsx file - Simplify UserMenu hydration handling - Fix CSS positioning with explicit margin properties to override NavItem styles and maintain right alignment - Add proper color styling to match other navigation items This resolves the recoverable hydration warnings in HeaderNav while preserving the visual layout and user experience.
Move ARIA disclosure attributes from generic SidebarContainer div to the semantic SidebarRoot <aside> element. This resolves the accessibility violation where aria-labelledby was used on an element without a proper semantic role. The <aside> element naturally supports ARIA attributes as a complementary landmark, ensuring both accessibility compliance and semantic correctness.
Add 1rem of top margin to the SidebarTitle component for desktop view only. This provides better visual spacing from the top of the sidebar while maintaining the existing mobile layout unchanged.
It's recommended to use this configuration because in next.js we need to manually add the fontawesome css in the _app.
This resolves a SSR hydration issue. Because we're directly importing the icon effectively as an SVG object it's part of the module code. This is a good practice we can use in other places that fontawesome is used.
Replace styled anchor tag with styled Link component to enable proper client-side routing, prefetching, and browser history management. Maintains all existing functionality including accessibility attributes, keyboard navigation, mobile menu closure, and Storybook compatibility. - Style Link component directly instead of using legacyBehavior - Preserve onNavigate callback for mobile menu integration - Handle both Enter and Space key navigation appropriately - All tests continue to pass with no breaking changes
Replace :focus with :focus-visible in SidebarNavItem to follow modern accessibility best practices. This hides focus rings on mouse clicks while preserving them for keyboard navigation, improving the user experience without compromising accessibility.
Add useRef hook and programmatic click handling to restore keyboard navigation functionality that was broken when switching from anchor tags to Next.js Link components. Now the keyboard handler only calls onNavigate() for side effects (like closing the mobile menu) while letting Next.js Link handle the actual navigation natively on Enter/Space key presses. This change maintains the same functionality while simplifying the code and following React/Next.js best practices.
- Force right alignment with !important margin overrides to overcome PrimaryNavigation's default equal margins - Add min-width and flex-shrink properties to prevent layout shifts - Separate hasMounted and isLoading checks for cleaner conditional logic - Improve hydration handling to reduce layout shifts and console warnings
jonathansick
force-pushed
the
tickets/DM-52394
branch
from
7cf26d6 to
e8c8542
Compare
Change parameter from 'navigation' to 'router' to fix Next.js router mocking in Storybook. The 'navigation' parameter is for App Router (app directory) while 'router' is for Pages Router (pages directory). This fix enables proper navigation active states in the Account, Access Tokens, and Sessions page stories.
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.
Introduce a new SidebarLayout component with desktop and mobile layouts, enhancing accessibility with keyboard navigation and ARIA attributes. Update architecture documentation and testing strategies, integrating vitest for unit tests alongside Storybook interaction tests.
Desktop view
Mobile view with expanded menu