-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Foundation for right sidebar with navigation history #24984
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
Draft
dennisoelkers
wants to merge
16
commits into
master
Choose a base branch
from
feat/right-sidebar-base
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
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
Implement a right sidebar component that can be triggered by any component in the main page content area. The sidebar renders next to the main content below the navigation bar. Features: - Context-based sidebar state management with RightSidebarProvider - Custom useRightSidebar hook for easy access from any component - Configurable sidebar width (default: 400px) - Dynamic content rendering with prop passing - Open/close/update content functionality - Integrated into app layout below navigation bar Architecture: - AppLayout: flex-column with Navigation at top and ContentRow below - ContentRow: flex-row with MainContentColumn and RightSidebar side-by-side - Sidebar conditionally renders when isOpen is true Added test button to WelcomePage for verification. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Address high-priority code review findings: 1. Type Safety Improvements: - Replace 'any' type with generic type parameter in RightSidebarContent - Add type safety for component props with generic <T> parameter - Maintain compile-time type checking for sidebar content 2. Accessibility Enhancements: - Add ESC key handler to close sidebar with keyboard - Add ARIA attributes: role="complementary", aria-label, aria-labelledby - Improve screen reader support with proper semantic markup 3. Remove Test Code: - Remove test button from Welcome.tsx - Clean production-ready implementation Technical details: - Context and Provider now use generic types for better type inference - ESC key listener properly cleaned up on unmount - ARIA attributes follow accessibility best practices Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changes: 1. Remove ESC key handler (no longer needed) 2. Style header to match PageHeader component: - Use h1 tag for title (matching PageHeader) - Use theme.fonts.size.h1 for consistent typography - Use 15px padding (matching PageHeader spacing) - Add gap: 10px between title and close button - Add flex: 1 to title for proper text overflow handling 3. Keep ARIA attributes for accessibility 4. Maintain type safety improvements from previous commit The sidebar header now has the same look and feel as the PageHeader component used throughout the application, providing a consistent user experience. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changes to scrolling behavior: 1. ContentRow now scrollable (overflow: auto) instead of hidden 2. Remove separate scrollbar from PageContent 3. Sidebar uses position: sticky to stay visible while content scrolls 4. Sidebar positioned at top: 0 within the ContentRow viewport Layout changes: - ContentRow: overflow: auto (single scrollbar for entire content area) - MainContentColumn: removed overflow: hidden, added min-width: 0 - PageContent: removed overflow: auto and height: 100% - Sidebar Container: - position: sticky with top: 0 - height: fit-content with max-height: 100vh - ContentArea has overflow-y: auto for internal scrolling Result: Main content scrolls within ContentRow, sidebar stays fixed in view at the top of the viewport. Both share the same scrolling container, eliminating the separate PageContent scrollbar. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Change sidebar height from fit-content/max-height: 100vh to height: 100vh to ensure the sidebar always fills the full viewport height. Changes: - Container: height: 100vh (was height: fit-content; max-height: 100vh) - Sidebar now takes up full screen height - ContentArea still scrollable if content exceeds available space Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace fixed height: 100vh with align-self: stretch to make the sidebar fill the ContentRow container height (viewport minus navigation). Changes: - Remove height: 100vh (was causing sidebar to be taller than available space) - Add align-self: stretch (fills ContentRow's height naturally) - Sidebar now correctly sized to viewport height minus navigation height - No scrolling of the sidebar container itself Since ContentRow is already sized with flex: 1 (taking remaining space after Navigation), the sidebar naturally fills that space without hardcoding any navigation height values. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Manual styling improvements: - Replace ContentHeadRow with bootstrap Row for better control - Add padding-right: 15px to Header for balanced spacing - Create StyledRow with zero margins to prevent extra spacing - Remove background-color and border styles from Container These changes improve the visual appearance and ensure proper spacing within the sidebar component. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add Jest + Testing Library tests covering: 1. RightSidebarProvider.test.tsx: - Default context values - Opening sidebar with content - Closing sidebar - Updating sidebar width - Uses userEvent for interactions 2. useRightSidebar.test.tsx: - Returns correct context value - Throws error when used outside provider - Opens sidebar with content - Closes sidebar - Updates content - Updates width - Uses act() and waitFor() for async state updates 3. RightSidebar.test.tsx: - Does not render when content is null - Renders sidebar when opened - Renders close button - Closes sidebar when close button clicked - Correct ARIA attributes - Renders custom components with props - Updates content when already open - Matches actual App.tsx usage with Context.Consumer All 17 tests passing. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Implements browser-like back/forward navigation for sidebar content, allowing users to navigate through previously viewed content. The history stack is limited to 20 items and follows standard browser behavior where opening new content mid-stack truncates forward history. Key additions: - History management using useReducer with 6 action types - Back/forward navigation buttons in sidebar header - useSidebarNavigation helper hook for easy integration - SidebarNavigationLink component for declarative navigation - Comprehensive test coverage (27 tests passing) All existing APIs remain backward compatible. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Enhances the example sidebar with multiple interconnected pages that demonstrate the navigation history system in action. Users can now explore a realistic multi-page sidebar experience with: - User Details page with links to settings and activity - User Settings page with sub-pages for notifications and security - Activity log page showing recent user actions - Notification and Security settings pages - Multiple navigation patterns (SidebarNavigationLink components and button clicks) The demo encourages users to try the back/forward navigation arrows to experience the browser-like history behavior. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
It was a pass-through over useRightSidebar that added no logic. Consumers now use useRightSidebar and openSidebar directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Welcome now uses useRightSidebar, which requires the provider. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use overflow shorthand and remove units from zero values in RightSidebar - Remove nesting selector from pseudo-classes in SidebarNavigationLink - Reorder example components to eliminate forward references - Remove redundant "Back to" links (sidebar back button handles this) - Remove flaky max history depth test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Description
Adds a generic right sidebar component that renders alongside page content, with a context-based API (
openSidebar,closeSidebar,updateContent) for opening arbitrary components inside it. Includes browser-like navigation history (back/forward) using auseReducer-based history stack with forward-truncation and a 20-item depth limit.The sidebar is sticky, fills the viewport height below the nav bar, and supports configurable width. A
SidebarNavigationLinkcomponent is provided for declarative in-sidebar navigation between content views./nocl WIP Feature
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: