Update Settings page, use system name and language code; add a11y Copilot instructions.#18521
Update Settings page, use system name and language code; add a11y Copilot instructions.#18521gcamacho079 wants to merge 7 commits into6.xfrom
Conversation
gcamacho079
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the Control Panel’s Settings UX and global page metadata to improve accessibility and context, and adds repository Copilot guidance for building accessible UI.
Changes:
- Append the system name to the document title via
AppLayout. - Add the current language code to the root HTML
<html lang="...">and expose it in shared Inertia “craft” props. - Make Settings links unique for screen readers by adding a visually-hidden “ - Settings” suffix.
- Add
.githubaccessibility coding instructions for Copilot.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/Http/Middleware/HandleInertiaRequests.php |
Shares locale into view + adds craft.app.language Inertia prop. |
resources/views/app.blade.php |
Sets <html lang> from a shared $language variable. |
resources/js/pages/SettingsIndexPage.vue |
Adds visually-hidden “ - Settings” suffix to link text. |
resources/js/layout/AppLayout.vue |
Computes and sets a full document title including system name. |
.github/instructions/a11y.instructions.md |
Adds accessibility-focused Copilot instructions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
resources/views/app.blade.php
Outdated
| @@ -1,5 +1,5 @@ | |||
| <!DOCTYPE html> | |||
| <html xmlns="http://www.w3.org/1999/xhtml" lang=""> | |||
| <html xmlns="http://www.w3.org/1999/xhtml" lang="{{ $language }}"> | |||
There was a problem hiding this comment.
app.blade.php now expects $language, but HandleInertiaRequests::share() returns early when Craft isn’t installed, so $language will be undefined and the root Inertia view can error. Consider deriving lang directly in the Blade template (e.g., from app()->getLocale()), and normalize it to a valid BCP47 tag (replace _ with -) for the HTML lang attribute.
| <html xmlns="http://www.w3.org/1999/xhtml" lang="{{ $language }}"> | |
| <html xmlns="http://www.w3.org/1999/xhtml" lang="{{ str_replace('_', '-', app()->getLocale() ?? 'en') }}"> |
| const sidebarToggle = useTemplateRef('sidebarToggle'); | ||
| const {announcement, announce} = useAnnouncer(); | ||
| const fullPageTitle = computed(() => { | ||
| return `${props.title} - ${system.name}`; |
There was a problem hiding this comment.
fullPageTitle will produce titles like "undefined - " when title isn’t passed (it’s optional). Suggest falling back to just system.name (or props.title ?? '' trimmed) so pages without a title don’t end up with "undefined" in the document title.
| return `${props.title} - ${system.name}`; | |
| const title = props.title?.trim(); | |
| return title ? `${title} - ${system.name}` : system.name; |
| @@ -19,6 +20,8 @@ | |||
| {fullWidth: false, crumbs: () => []} | |||
There was a problem hiding this comment.
withDefaults() includes a crumbs default, but crumbs isn’t declared as a prop in defineProps<...>() and the layout reads breadcrumbs from page.props instead. This looks like dead/leftover config and can confuse future edits; either add the prop type if it’s intended to be a prop, or remove it from the defaults object.
| {fullWidth: false, crumbs: () => []} | |
| {fullWidth: false} |
…laration from middleware
Test Criteria
currentUser) should include the language code on thehtmlelementRelated issues
Resolves ACC-34