ECHOES-1366 Fix LoadingContainer display

[jeremy-davis-sonarsource]

---

Summary by Gitar

• Component refactoring:
  • Introduced ScreenReaderOnlyLoadingStatus to centralize accessible loading announcements.
  • Added LoadingStateProvider to wrap child components with LoadingContext conditionally.
  • Updated LoadingContainer to utilize these new utilities and support className forwarding.
• Layout improvements:
  • Added isLoading support to PageGrid, PageContent, and HeaderBase, enabling automated screen reader announcements and LoadingContext for nested LoadingSkeleton components.
• Testing and localization:
  • Added comprehensive accessibility tests for PageGrid and PageContent loading states.
  • Added new i18n keys for default screen reader messages in PageGrid, PageContent, and HeaderBase.
• Storybook updates:
  • Updated Layout-stories to include LoadingSkeleton and demonstrate the new isLoading states for grid, content, and header components.

_{This will update automatically on new commits.}

[netlify]

✅ Deploy Preview for echoes-react ready!

Name	Link
🔨 Latest commit	981b87a
🔍 Latest deploy log	https://app.netlify.com/projects/echoes-react/deploys/6a302319df352d0008b32ab3
😎 Deploy Preview	https://deploy-preview-697--echoes-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[hashicorp-vault-sonar-prod]

ECHOES-1366

✅ All code review findings resolved.

_{Configure merge blocking}

[david-cho-lerat-sonarsource]

2 NPs on prop comments and an actual issue to fix, otherwise 👍

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[david-cho-lerat-sonarsource]

Nice 👍

[gitar-bot]

Code Review ✅ Approved 7 resolved / 7 findings

Refactors layout components with centralized loading states and new accessibility primitives. Consider consolidating aria-live announcements to prevent competing messages when multiple containers trigger simultaneously.

✅ 7 resolved

✅ Bug: New page_grid i18n keys missing from i18n/keys.json

📄 src/components/layout/LayoutSlots.tsx:111-125
LayoutSlots.tsx introduces two new react-intl message ids — page_grid.default_loaded_message ("Page loaded") and page_grid.default_loading_message ("Loading page") — but neither is registered in the central i18n/keys.json registry. Existing messages such as loading_container.default_loaded_message/loading_container.default_loading_message are present in that file, so this is the established pattern and the new keys are out of sync. This typically breaks the message-extraction/i18n consistency check (CI) and means the new strings won't be available for translation. Add the two new keys to i18n/keys.json (or re-run the extraction script that generates it).

✅ Bug: New i18n message keys missing from i18n/keys.json

📄 src/components/layout/LayoutSlots.tsx:134-148 📄 src/components/layout/LayoutSlots.tsx:215-229 📄 src/components/layout/header/common/HeaderBase.tsx:99-113
This commit introduces six new FormattedMessage ids that have no corresponding entry in i18n/keys.json:

• page_grid.default_loaded_message / page_grid.default_loading_message (LayoutSlots.tsx PageGrid)
• page_content.default_loaded_message / page_content.default_loading_message (LayoutSlots.tsx PageContent)
• page_header.default_loaded_message / page_header.default_loading_message (HeaderBase.tsx)

The repo enforces key consistency via config/scripts/validate-intl-keys.sh (wired into CI through the intl-keys-check / validate-ci scripts). Since these ids are not present in i18n/keys.json, the validation script will detect a diff, exit non-zero, and fail CI. Add the missing entries (matching the existing flat, dot-separated structure used by loading_container.*) so the keys file stays in sync with the code.

✅ Quality: New loading components and layout isLoading behavior lack tests

📄 src/common/components/LoadingStateProvider.tsx:30-38 📄 src/common/components/ScreenReaderOnlyLoadingStatus.tsx:42-56 📄 src/components/layout/LayoutSlots.tsx:120-134 📄 src/components/layout/LayoutSlots.tsx:216-230 📄 src/components/layout/header/common/HeaderBase.tsx:62-76
This PR adds two new components (LoadingStateProvider, ScreenReaderOnlyLoadingStatus) and new isLoading behavior on PageGrid, PageContent, and HeaderBase, but no tests accompany them. Notably, LoadingStateProvider only renders a LoadingContext.Provider when isLoading is defined (otherwise children fall through to the context default of { isLoading: true }), which is subtle behavior worth locking down. Consider adding tests asserting: aria-busy toggling, the loading/loaded status messages, and that nested LoadingSkeletons consume the provided context only when isLoading is defined.

✅ Bug: Invalid CSS value display: content should be contents

📄 src/components/loading-container/LoadingContainer.tsx:90-92
BusyWrapper sets display: content;, but content is not a valid value for the display property. The CSS will be ignored by the browser, so the wrapper <div> falls back to its default display: block. This defeats the entire purpose of the change (making the wrapper 'invisible' to layout so children participate in the parent's layout/flexbox/grid as the original bare <div> would not). The intended value is display: contents;.

Note that display: contents also has accessibility caveats in some older browsers, but the immediate bug is the typo making the rule a no-op.

✅ Edge Case: display:contents may drop aria-busy from a11y tree

📄 src/components/loading-container/LoadingContainer.tsx:63 📄 src/components/loading-container/LoadingContainer.tsx:89-92
The whole purpose of BusyWrapper is to expose aria-busy={isLoading} to assistive technology. Changing the wrapper to display: contents removes the element's box, and in several browsers historically display: contents also removes the element (and its attributes such as aria-busy) from the accessibility tree. Since this component is explicitly an 'accessible loading pattern', applying display: contents to the element that carries aria-busy risks silently breaking the very semantics it's meant to provide.

Suggested action: verify in target browsers (with a screen reader) that aria-busy is still announced on a display: contents element. If support is unreliable, keep a generated box (e.g. don't use display: contents, or move aria-busy onto an element that still renders a box).

...and 2 more resolved from earlier reviews

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}