Repository navigation
[Fix] Settings and analytics lack a main content landmark - #3495
Open
roomote-roomote[bot] wants to merge 1 commit into
Open
roomote-roomote[bot] wants to merge 1 commit into
roomote-roomote[bot] wants to merge 1 commit into
Conversation
roomote-roomote
Bot
requested review from
brunobergher,
daniel-lxs and
mrubens
as code owners
October 16, 2026 05:46
Contributor
|
No code issues found. See task Reviewed both changed files and the settings/analytics callers and enclosing layout. The main landmark contains the heading and content, leaves desktop section navigation outside, and preserves the existing layout and scrolling classes. The regression assertions cover that boundary. Current-head lint, typecheck, and Knip checks passed; tests, app Docker build, and JavaScript/TypeScript CodeQL analysis were still running when inspected. Repository validation suites were not rerun locally. Reviewed 06023dc |
This was referenced Oct 16, 2026
This branch has not been deployed
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
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.
What changed
Expose the existing settings and analytics content wrapper as a main landmark, with the page heading and controls inside it and desktop section navigation outside it. Preserve its existing styles and scroll behavior.
Why this change was made
These pages have no main landmark on current develop, preventing assistive-technology landmark navigation from finding their primary content. Fix the shared settings/analytics shell rather than the global shell, where adding a main would nest the existing Sessions landmark.
Impact
Settings and analytics now provide one main landmark without adding UI, state or dependencies. The regression assertion covers the semantic boundary between main content and navigation.
Actual desktop/mobile settings navigation and analytics view selection were exercised. Browser DOM/accessibility checks report one main around the correct heading/content, the missing-main axe finding is gone on inspected pages, and Sessions retains exactly one main. Four focused shell/analytics tests and repository lint, fast typecheck and Knip gates pass. Physical screen-reader speech is not verified; this does not claim every accessibility finding is resolved (desktop analytics still reports a separate navigation-landmark uniqueness finding). The screenshots establish preserved representative layouts, not semantics by pixels.
Screenshots
Genuine local authenticated pages with no simulated state:
Related PRs