Skip to content

[WT-1677] Conditional block heading levels - #1794

Open
maribedran wants to merge 5 commits into
mainfrom
WT-1677-heading-levels
Open

[WT-1677] Conditional block heading levels#1794
maribedran wants to merge 5 commits into
mainfrom
WT-1677-heading-levels

Conversation

@maribedran

Copy link
Copy Markdown
Collaborator

One-line summary

Use h1 for all sequential conditional blocks at the top of a page so all users get served a page with the correct heading hierarchy.

Significant changes and points to review

  • Update the templates of all free-form page types (Free Form, What's New, and Smart Window Explainer) to consider conditional display settings when determining block heading levels. If a page leads with sequential conditional blocks, all of them use h1s
  • Fix bug that considered the Notification block when determining if a block has a heading (the block does have a field called headline, but it isn't rendered as a heading element)
  • Show a warning when an editor saves a page with leading conditional blocks

Issue / Bugzilla link

https://mozilla-hub.atlassian.net/browse/WT-1677

Testing

Create free-form pages with different combinations of blocks with headings such that:

  • Leading with a Notification, with or without a headline, doesn't affect the following blocks' heading levels
  • Leading with a block that renders a heading renders that block with an h1 and the following ones with h2
  • Leading with blocks that render headings and have conditions renders all of those with h1, followed by h2
  • Saving a page with leading conditional blocks displays a warning

Unconditional:
image

Conditional:
image

Warning:
image

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.59459% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.09%. Comparing base (dee1884) to head (4c74156).

Files with missing lines Patch % Lines
springfield/cms/wagtail_hooks.py 93.75% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1794      +/-   ##
==========================================
+ Coverage   90.08%   90.09%   +0.01%     
==========================================
  Files         182      182              
  Lines       13219    13256      +37     
==========================================
+ Hits        11908    11943      +35     
- Misses       1311     1313       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The behavior change is well-scoped and backed by targeted tests; only a small docstring clarification was noted.

Pull request overview

This PR updates how free-form page templates compute heading levels so that leading blocks with conditional display rules can each render as h1 (ensuring the first heading a visitor actually sees is an h1), and adds a Wagtail admin warning to flag pages that start with multiple conditional heading blocks.

Changes:

  • Add a has_conditions property to conditional-display values and use it from templates + hooks.
  • Update free-form templates to treat the “leading conditional run” of heading blocks as h1s, while subsequent headings follow normal hierarchy (h2+).
  • Add a Wagtail after_create_page/after_edit_page warning plus tests covering both the warning and the heading-level behavior (including excluding Notification headlines).

Note: This review was performed following the repository’s Copilot/AGENTS.md review guidance.

File summaries
File Description
springfield/cms/blocks.py Adds ConditionalDisplayValue.has_conditions and wires it into ConditionalDisplayBlock via value_class.
springfield/cms/wagtail_hooks.py Adds helpers to detect leading conditional heading blocks and warns editors via Django messages.
springfield/cms/templates/cms/free_form_page2026.html Adjusts heading-level selection to keep leading conditional heading blocks at h1.
springfield/cms/templates/cms/smart_window_explainer_page.html Mirrors the conditional-run heading-level logic for the explainer template.
springfield/cms/tests/test_pages.py Adds tests verifying the editor warning hook behavior (including Notification preceding blocks).
springfield/cms/tests/test_blocks.py Adds end-to-end HTML assertions ensuring heading hierarchy is correct across FreeForm/What’s New/Explainer pages.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +788 to +789
The run ends at the first heading block shown to everyone, since that block takes
the page's only h1 and every heading block after it drops to h2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants