fix(ui): give the admin area a main landmark - #1589
Closed
culfin wants to merge 1 commit into
Closed
Conversation
Follow-up to apache#1587, which covered the two layouts that carry the side navigation. The admin area has a layout of its own, sits directly under `pages/Layout` in the router, and was therefore not included. Same two changes as there: the wrapper around `Outlet` becomes `main`, the sidebar container becomes `nav`. Class names untouched, so nothing moves. It covers 28 routes. One correction to what apache#1587 says about this: it names `Users/Settings` and `Legal` as also needing their own landmark. They do not — both are nested inside `SideNavLayout`, so they already receive the one added there. Adding a second would be worse than none, since only one `main` per page is valid. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 24, 2026
Author
|
Folded into #1587, which now covers every route rather than a third of them. Nothing is dropped; the commits are on that branch. Closing to keep the queue readable. |
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
1 participant
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.
Follow-up to #1587, which covered the two layouts that carry the side
navigation. The admin area has a layout of its own and sits directly under
pages/Layoutin the router, so it was not included there. It covers 28routes.
Same two changes as in #1587: the wrapper around
Outletbecomesmain, thesidebar container becomes
nav. Class names untouched, so nothing moves.A correction to #1587
That PR names
Users/SettingsandLegalas also needing a landmark of theirown. They do not. Both are nested inside
SideNavLayout:Adding a second
mainthere would be worse than none, since only one per pageis valid. Sorry for the noise in the other description.
Still open
Twenty routes hang directly under
pages/Layoutwith no layout of their own —sign-in, registration, the error pages. Giving each one a landmark means either
twenty edits or a small
PlainLayoutwrapper in the router. That is adifferent kind of change and belongs in its own PR; I am happy to send it if
you would like it.
🤖 Generated with Claude Code