Skip to content

fix(side-panel): fix tabbed navigation#2044

Open
robertwilde wants to merge 1 commit into
mainfrom
fix/side-panel/tabbed-nav
Open

fix(side-panel): fix tabbed navigation#2044
robertwilde wants to merge 1 commit into
mainfrom
fix/side-panel/tabbed-nav

Conversation

@robertwilde

@robertwilde robertwilde commented May 12, 2026

Copy link
Copy Markdown
Collaborator

Closes #1841

Fix keyboard tab order and focus management in si-side-panel. Main content is now rendered before .side-panel in the DOM so tab order matches visual layout (app header before side panel). cdkTrapFocus prevents focus escaping when the panel is open as an overlay. An inert attribute eliminates ghost tab stops during the close animation in non-collapsible over mode. si-side-panel-actions is moved to the end of the DOM and repositioned visually with CSS order: -1, so collapsed accordion headers are tabbed before action buttons, matching their visual order in the icon-strip state.


Documentation.
Examples.
Dashboards Demo.
Playwright report.

Coverage Reports:

Code Coverage

@robertwilde robertwilde force-pushed the fix/side-panel/tabbed-nav branch from 4cc236a to 8c6d7fb Compare May 12, 2026 16:14

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the side panel component's accessibility and layout by implementing focus trapping with CdkTrapFocus and managing the inert attribute based on the panel's state. Additionally, the template structure for side panel actions was adjusted. Feedback was provided regarding a potentially unintended CSS rule for .rpanel-statusactions that might have been included by mistake.

Comment on lines +84 to +86
.rpanel-statusactions {
order: -1;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This CSS rule appears to be unintended. It applies order: -1 to .rpanel-statusactions, which moves the status actions block to the top of its flex container. The pull request description justifies reordering si-side-panel-actions for accessibility reasons, but does not mention status actions. This might be a copy-paste artifact. If this reordering is not intentional, this rule should be removed to prevent unexpected layout changes.

@robertwilde robertwilde force-pushed the fix/side-panel/tabbed-nav branch 2 times, most recently from fcac77d to 7c6742c Compare May 12, 2026 17:24
@robertwilde robertwilde marked this pull request as ready for review May 12, 2026 17:41
@robertwilde robertwilde requested review from a team as code owners May 12, 2026 17:41
@robertwilde

Copy link
Copy Markdown
Collaborator Author

@dr-itz @chintankavathia I thought I'd give this a shot.

@dr-itz

dr-itz commented May 12, 2026

Copy link
Copy Markdown
Member

status-action tab-order when the panel is expanded makes no sense to me. Before the collapsed order was weird, now it's the expanded order.

@panch1739 @hbxes why do we move the status actions to the bottom when collapsed? I mean if these are important enough to be on top in expanded state, then why shouldn't the be on top in collapsed state as well?

I.e. expanded:
Screenshot 2026-05-12 at 21 49 21

collapsed, current behaviour:

Screenshot 2026-05-12 at 21 49 46

collapsed, but keep order (proposal):
Screenshot 2026-05-12 at 21 49 40

@robertwilde

robertwilde commented May 13, 2026

Copy link
Copy Markdown
Collaborator Author

In general I find the behavior that the icons disappear when the side panel is expanded, confusing. If the actions are reordered, keeping the icons could help.

@robertwilde robertwilde force-pushed the fix/side-panel/tabbed-nav branch from 3d0f6ff to 6086954 Compare May 21, 2026 13:09
@spike-rabbit spike-rabbit added this to the 51.x milestone Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tab order of layout is broken when using side panel with application header

3 participants