Skip to content

Keep one H1 on hasVisibleHeading pages while they load - #15490

Open
rtibblesbot wants to merge 1 commit into
learningequality:developfrom
rtibblesbot:issue-15467-dfae6a
Open

rtibblesbot wants to merge 1 commit into
learningequality:developfrom
rtibblesbot:issue-15467-dfae6a

Conversation

@rtibblesbot

@rtibblesbot rtibblesbot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • AppBarPage and ImmersivePage render a hidden <h1> until a hasVisibleHeading page's own <h1> appears in #main.

References

Fixes #15467. Heading focus: #15485.

Reviewer guidance

Code review

  • usePageHeading observes #main only while the innermost registration sets hasVisibleHeading.
  • The shell's own hidden <h1> is excluded by template ref, so it cannot remove itself.
  • Risk: focus on the hidden H1 is lost when it unmounts.

QA steps

Setup: Chrome, a screen reader's headings list (NVDA Insert+F7, VoiceOver rotor) or the HeadingsMap extension, DevTools > Network > Slow 3G. Sign in as admin; import a channel with a course.

Where While loading After load
Coach, from a class page back to the class list one hidden H1 "Classes" one visible H1 "Classes"
Coach > a class > Courses > a course one hidden H1, the course title one visible H1, the course title
Device > Info one hidden H1 "Device info" one visible H1 "Device info"
Learn > Home; Facility > Data one hidden H1 one hidden H1, as on develop

Evidence

API delayed 3 s; overlay lists every <h1>.

Loading Loaded
Coach Classes loading: one hidden H1 Coach Classes loaded: one visible H1
coach-classes-reload.webm

AI usage

Claude Code wrote the change and its Jest tests.

🤖 Generated with Claude Code


Written by rtibblesbot, an LLM-based coding agent.

@rtibblesbot

🟡 Held for manual QA (🧪 Needs QA)

Last updated: 2026-10-30 16:30 UTC

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

npm Package Versions

Warning

The following packages have changed files but no version bump:

Package Version Changed files
kolibri 0.18.0 4

If these changes affect published code, consider bumping the version.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

@rtibblesbot
rtibblesbot marked this pull request as ready for review October 3, 2026 20:04
@rtibblesbot
rtibblesbot requested review from a team and leq-bot October 3, 2026 20:04
@rtibblesbot
rtibblesbot force-pushed the issue-15467-dfae6a branch 2 times, most recently from 1b65fd3 to 1130012 Compare October 4, 2026 21:41
@rtibbles rtibbles self-assigned this Oct 4, 2026

@rtibbles rtibbles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks fine from a quick read through, but please write better QA guidance, you can do better than this.

AppBarPage and ImmersivePage watch #main and show the registered title
as a hidden <h1> whenever a hasVisibleHeading page has no <h1> there,
so loading states keep exactly one H1.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Rewrote the PR body; the old QA steps and evidence described sibling PRs' flows.

  • QA steps: a Where | While loading | After load table for pages whose <h1> renders after load, plus two develop-identical controls; each row except the course page checked live with the API delayed 3 s.
  • Same mistake elsewhere in the body: 4 sections — the risk bullets, the 49 evidence captures, the Deviations table, the Register Coach page titles with usePageTitle #15476 reference. All replaced or removed.
  • Rebased on develop: Register Coach page titles with usePageTitle #15476 has merged, so Coach course pages now register hasVisibleHeading and the Deviations row no longer holds.
  • Course page not captured: the QA seed has no course channel. Evidence is the Coach class list instead.

@rtibbles rtibbles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good, QA confirmation needed.

@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Holding off on changes while this PR is in 🧪 Needs QA — the branch will not move while it is being tested. Questions still get answered.

Change the board status when QA is done and I'll pick up any outstanding review feedback, CI failures and conflicts on the next cycle.


Written by rtibblesbot, an LLM-based coding agent.

1 similar comment
@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Holding off on changes while this PR is in 🧪 Needs QA — the branch will not move while it is being tested. Questions still get answered.

Change the board status when QA is done and I'll pick up any outstanding review feedback, CI failures and conflicts on the next cycle.


Written by rtibblesbot, an LLM-based coding agent.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pages with a visible H1 keep exactly one H1 while loading

2 participants