va-maintenance-banner: Prevent additional slot from rendering in light DOM by RyanMunsch · Pull Request #1988 · department-of-veterans-affairs/component-library

RyanMunsch · 2026-02-12T15:07:00Z

Chromatic

https://4649-maintenance-banner-announcement--65a6e2ed2314f7b8f98609d8.chromatic.com

Description

This pull request refactors the va-maintenance-banner web component to improve its logic for rendering and accessibility. The main focus is on determining when and how the maintenance banner should be displayed, ensuring unused content is removed for screen readers, and simplifying conditional rendering. Additionally, related test cases are updated to match the new logic.

Component logic and rendering improvements:

Added a componentWillLoad lifecycle method to centralize logic for:
- Determining if the banner should render based on dates and local storage.
- Setting a flag (isWarning) to distinguish between warning and error states.
- Removing the unused slot (warn-content or maintenance-content) from the DOM for accessibility. [1] [2]
Refactored the render method to:
- Use the new preventRender flag to decide whether to render the banner.
- Simplify and clarify the logic for which slot content to display based on isWarning. [1] [2] [3]

Accessibility improvements:

Ensured that only the relevant slot (warn-content or maintenance-content) is present in the DOM, preventing confusion for screen readers and assistive technologies.

Test updates:

Updated end-to-end tests to match the new slot removal logic, ensuring only the appropriate content is present for each banner state. [1] [2] [3]

Code cleanup:

Adjusted imports to include the @Element decorator for DOM manipulation.

Screenshots

Before

After

Testing and review

Approvals

See the QA Checklists section below for suggested approvals. Use your best judgment if additional reviews are needed. When in doubt, request a review.

Approval groups

Add approval groups to the PR as needed:

Engineering: platform-design-system-fe
Accessibility: platform-design-system-a11y
Design: platform-design-system-designer

QA checklists

Use the QA checklists below as guides, not rules. Not all checklists will apply to every PR but there could be some overlap.

In all scenarios, changes should be fully tested by the author and verified by the reviewer(s); functionality, responsiveness, etc.

✨ New Component Added

🌱 New Component Variation Added

The PR has the minor label
The variation matches its Figma design.
Any new properties, custom events, or utility functions have e2e and/or unit tests
A new story has been added to the component's existing Storybook page
Any Chromatic UI snapshot changes have been accepted by a design reviewer
Tested in vets-website using Verdaccio
Engineering has approved the PR
Design has approved the PR

🐞 Component Fix

The PR has the patch label
Any new properties, custom events, or utility functions have e2e and/or unit tests
Any markup changes are evaluated for impact on vets-website.
- Will any vets-website tests fail from the change?
Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
Engineering has approved the PR

♿️ Component Fix - Accessibility

The PR has the patch label
Any new properties, custom events, or utility functions have e2e and/or unit tests
Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
Engineering has approved the PR
Accessibility has approved the PR

🚨 Component Fix - Breaking API Change

The PR has the major label
vets-website and content-build have been evaluated to determine the impact of the breaking change
Any new properties, custom events, or utility functions have e2e and/or unit tests
Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
Tested in vets-website using Verdaccio
Engineering has approved the PR

🔧 Component Update - Non-Breaking API Change

The PR has the minor label
Any new properties, custom events, or utility functions have e2e and/or unit tests
Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
Engineering has approved the PR

📖 Storybook Update

The PR has the ignore-for-release label
Any Chromatic UI snapshot changes have been reviewed and approved by a designer if necessary
Engineering has approved the PR

🎨 CSS-Library Update

The PR has the css-library label
vets-website and content-build have been checked to determine the impact of any breaking changes
Engineering has approved the PR

Also refactor approach to move logic out of component's render method.

Copilot

Pull request overview

This pull request refactors the va-maintenance-banner web component to improve its rendering logic, code organization, and accessibility. The component displays maintenance or warning banners on VA.gov based on date/time conditions and user interactions.

Changes:

Introduced componentWillLoad lifecycle method to centralize banner display logic, determine warning vs error state, and remove unused slots for accessibility
Refactored the render method to use cleaner conditional logic with pre-computed flags instead of nested conditions
Updated e2e tests to reflect that only the active slot remains in the light DOM

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
packages/web-components/src/components/va-maintenance-banner/va-maintenance-banner.tsx	Added componentWillLoad method for early logic execution, added isWarning and preventRender flags, implemented slot removal for accessibility, and refactored render method for clearer conditional logic
packages/web-components/src/components/va-maintenance-banner/test/va-maintenance-banner.e2e.ts	Updated test expectations to verify only the appropriate slot (warn-content or maintenance-content) remains in the light DOM based on banner state

Copilot · 2026-02-13T14:21:45Z

+    if (
+      isDateBefore(now, new Date(upcomingWarnStartDateTime)) ||
+      isDateAfter(now, new Date(maintenanceEndDateTime)) ||
+      window.localStorage.getItem('MAINTENANCE_BANNER') === bannerId
+    ) {
+      this.preventRender = true;
+    }


The refactored render method improves code readability by removing the nested conditional check for localStorage. However, there's now a subtle behavior change: in the old code, the localStorage check was done at render time (which happens after componentWillLoad), but now it's done in componentWillLoad. If the component is constructed and componentWillLoad runs, but the user dismisses the banner via some other mechanism before render happens, this could lead to inconsistent state. While this is a very unlikely edge case, it's worth noting the timing difference.

For @department-of-veterans-affairs/platform-design-system-fe, I'm not sure that this is actually an issue. I was testing setting/removing that banner ID value from local storage in the Chrome devtools console and it has the same behavior here on the Chromatic deploy that it does on production.

Copilot · 2026-02-13T14:21:46Z

        <div slot="maintenance-content">
          Weâ€™re working on VA.gov right now. If you have trouble signing in or using tools, check back after weâ€™re finished. Thank you for your patience.
        </div>
-        <div slot="warn-content">
-          Weâ€™ll be doing some work on VA.gov. The maintenance will last X hours. During that time, you wonâ€™t be able to sign in or use tools.
-        </div>
      </va-maintenance-banner>
    `);
  });


The tests for non-rendering scenarios (when the banner is expired or before the warning start date) use attribute-based content rather than slotted content. This means there's no test coverage for verifying the slot removal behavior when preventRender is true. Consider adding a test that uses slotted content (like the rendering tests do) to verify that slot removal works correctly even when the component doesn't ultimately render.

Copilot · 2026-02-13T14:21:46Z

+  isWarning: boolean = false;
+  preventRender: boolean = false;


The property preventRender is declared with an initial value of false but is never reset back to false after being set to true. This could cause issues if the component instance is reused or if props change dynamically. Consider using @State() decorator for this property since it affects rendering, or ensure it's properly managed through the component lifecycle when props change.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

amyleadem

Thanks for working on this @RyanMunsch. I was able to confirm that the component successfully leaves only the appropriate warning OR maintenance slot in the light DOM when one of them is active.

However, I found that when the date is outside both the maintenance and warning windows (for example, if the windows are in the past), the component leaves everything in the DOM.

Here are the values I used (which should match the current implementation on va.gov):

'upcoming-warn-start-date-time': "Mon Jun 08 2020 19:00:00 GMT-0700 (Mountain Standard Time)"
'maintenance-start-date-time': "Tue Jun 09 2020 07:00:00 GMT-0700 (Mountain Standard Time)"
'maintenance-end-date-time': "Wed Jun 10 2020 07:00:00 GMT-0700 (Mountain Standard Time)"

Can you look into methods for preventing that content from being accessible when there is no active maintenance or warning alerts?

Let me know if I am misunderstanding anything here!

RyanMunsch · 2026-02-20T17:57:28Z

Good catch, @amyleadem. I pushed an update that should resolve that issue. Good for another look when you have the chance.

amyleadem

This is looking good to me from an accessibility perspective! I reviewed the rendered code and found that the component now removes everything from the light DOM except for what currently applies to the component.

I had a couple of open questions related to refining the logic and code. Let me know if you have any questions.

RyanMunsch · 2026-02-23T18:41:41Z

Thanks for the suggestions, @amyleadem. This is ready for another round when you are.

amyleadem

LGTM! I had one open question about some possible redundant logic, but I don't think it is hurting anything. Mostly just wanting to understand.

Since there are few rounds of changes here, I'd like to one more engineering review/re-review before merge. @jamigibbs @powellkerry

amyleadem · 2026-02-23T19:38:59Z

+      warningStartInFuture ||
+      maintenanceEndInPast ||
+      bothWindowsInPast ||
+      bothWindowsInFuture ||


question(non-blocking): Why are bothWindowsInPast and bothWindowsInFuture needed? Curious if I am missing a configuration gap that this is helping with, or if they are redundant. If they are doing some safety checks, can we add a comment explaining?

Wow, this should have been obvious to me but just now realizing what you meant in your previous round of comments. bothWindowsInFuture is indeed not necessary due to warningStartInFuture alone being enough to prevent render. Thank you, @amyleadem!

jamigibbs

Thanks, Ryan! 🚀

Update logic to prevent unused slot in light DOM

08f770e

Also refactor approach to move logic out of component's render method.

RyanMunsch added the patch Patch change in semantic versioning label Feb 12, 2026

RyanMunsch marked this pull request as ready for review February 12, 2026 15:23

RyanMunsch requested a review from a team as a code owner February 12, 2026 15:23

RyanMunsch requested review from a team February 12, 2026 15:39

jamigibbs requested a review from Copilot February 13, 2026 14:17

Copilot started reviewing on behalf of jamigibbs February 13, 2026 14:18 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

RyanMunsch and others added 2 commits February 13, 2026 11:29

Fix inconsitent spacing

e88b4c3

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Prevent slot from being removed if component should not render

2a13276

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

powellkerry approved these changes Feb 16, 2026

View reviewed changes

jamigibbs approved these changes Feb 17, 2026

View reviewed changes

amyleadem requested changes Feb 19, 2026

View reviewed changes

Prevent slots from rendering when current date is outside all windows

45812d6

RyanMunsch requested a review from amyleadem February 20, 2026 17:57

amyleadem requested changes Feb 20, 2026

View reviewed changes

RyanMunsch added 3 commits February 23, 2026 13:17

Update test to use hard coded date strings

2847f6f

Clean up componentWillLoad logic

40948b5

Remove unused component property

c3f43d1

RyanMunsch requested a review from amyleadem February 23, 2026 18:41

amyleadem approved these changes Feb 23, 2026

View reviewed changes

Remove unnecessary check for "both windows in future"

62cfcd5

RyanMunsch requested a review from jamigibbs February 23, 2026 20:33

jamigibbs approved these changes Feb 23, 2026

View reviewed changes

RyanMunsch added 2 commits February 24, 2026 10:49

Merge branch 'main' into 4649-maintenance-banner-announcement

e7f1dd7

Merge branch 'main' into 4649-maintenance-banner-announcement

f6b2fd3

RyanMunsch merged commit 229dd94 into main Feb 24, 2026
7 checks passed

RyanMunsch deleted the 4649-maintenance-banner-announcement branch February 24, 2026 16:20

RyanMunsch mentioned this pull request Feb 24, 2026

Update package versions for v56.0.0 release #2011

Merged

44 tasks

Conversation

RyanMunsch commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Chromatic

Description

Related tickets and links

Screenshots

Before

After

Testing and review

Approvals

QA checklists

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

RyanMunsch Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amyleadem left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RyanMunsch commented Feb 20, 2026

Uh oh!

amyleadem left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RyanMunsch commented Feb 23, 2026

Uh oh!

amyleadem left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amyleadem Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

RyanMunsch Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

jamigibbs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

RyanMunsch commented Feb 12, 2026 •

edited

Loading

amyleadem left a comment •

edited

Loading

amyleadem left a comment •

edited

Loading

amyleadem left a comment •

edited

Loading