Conversation
… a semantic element
There was a problem hiding this comment.
Pull request overview
Implements global “skip links” for improved keyboard/accessibility navigation across the CP, with index pages contributing additional skip targets and breadcrumb navigation moved outside the main landmark so it can be bypassed.
Changes:
- Add global skip-link rendering in
AppLayout, withIndexLayoutinjecting extra skip links for secondary nav/content. - Add/adjust ARIA landmark labels (Primary/Secondary/Breadcrumbs) and move breadcrumbs outside
#main. - Update skip-link styling and add missing translations.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| resources/translations/en/app.php | Adds translation strings for “Secondary” and “Skip to secondary navigation”. |
| resources/js/pages/SettingsSitesIndex.vue | Removes extra <nav> wrapper in interior-nav slot (avoids nested nav now that layout provides it). |
| resources/js/layout/IndexLayout.vue | Adds secondary-nav/content skip-link targets and wraps interior nav in a labeled <nav> with focus target IDs. |
| resources/js/layout/AppLayout.vue | Introduces global skip links, adds additionalSkipLinks prop, and moves breadcrumbs outside the main container. |
| resources/js/components/CpSidebar.vue | Adds an aria-label (“Primary”) to the main sidebar navigation landmark. |
| resources/js/components/Breadcrumbs.vue | Wraps breadcrumbs list in a labeled <nav> landmark (“Breadcrumbs”). |
| resources/build/legacy.js | Bundled output updated (import symbol changes). |
| resources/build/assets/cp.css | Bundled CSS updated to reflect skip-link styling changes. |
| resources/build/assets/AppLayout.css | Bundled CSS updated for AppLayout scope hash changes. |
| resources/build/Updater.js | Bundled output updated (import symbol changes). |
| resources/build/SettingsSitesIndex.js | Bundled output updated for SettingsSitesIndex changes. |
| resources/build/SettingsSitesEdit.js | Bundled output updated (import symbol changes). |
| resources/build/SettingsIndexPage.js | Bundled output updated (import symbol changes). |
| resources/build/SettingsGeneralPage.js | Bundled output updated (import symbol changes). |
| resources/build/Install.js | Bundled output updated (import symbol changes). |
| resources/build/IndexLayout.js | Bundled output updated for IndexLayout changes. |
| resources/build/DeleteSiteModal.vue_vue_type_script_setup_true_lang.js | Bundled output updated (import symbol changes). |
| resources/build/CpQueueIndicator.js | Bundled output updated (internal symbol changes). |
| resources/build/CalloutReadOnly.vue_vue_type_script_setup_true_lang.js | Bundled output updated (import symbol changes). |
| resources/build/AppLayout.js | Bundled output updated, but currently contains debug/placeholder content that doesn’t match source. |
| packages/craftcms-cp/src/styles/cp.css | Updates .skip-link styling and adds .skip-link--global positioning variables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
resources/js/layout/AppLayout.vue
Outdated
| fullWidth?: boolean; | ||
| additionalSkipLinks?: Array<{label: string; url: string;}>; | ||
| }>(), | ||
| {fullWidth: false, crumbs: () => []} |
There was a problem hiding this comment.
withDefaults() is providing a default for crumbs, but crumbs is not a defined prop in defineProps<...>() (crumbs comes from page.props). This will at minimum confuse the props contract and can cause TS macro/type errors. Remove the crumbs default (or add a crumbs prop if that was intended).
| {fullWidth: false, crumbs: () => []} | |
| {fullWidth: false} |
Description
Re-implements skip links with a few updates/improvements
AppLayouthas a single skip link by default, butIndexLayoutpushes additional skip links via anadditionalSkipLinksprop.#maincontainer, so it can be bypassed via the “Skip to main” link.To test
Related issues
Resolves ACC-29