Dunk.dirty dialog by duncanuszkay-d2l · Pull Request #6798 · BrightspaceUI/core

duncanuszkay-d2l · 2026-04-14T14:43:28Z

https://desire2learn.atlassian.net/browse/NTNGL-5471?atlOrigin=eyJpIjoiODJjZWQ3NzZmNjE0NDc1MTgxOTEwMTY5MGUzNDNkZTAiLCJwIjoiaiJ9

We're adding a dirty state to the table component to act as an intermediate state between clean and loading while the user changes their selections.

I've split this PR into two commits:

Move from a shown boolean to a dataState enum, to allow the addition of a third state
Add in the dirty state dialog

Demo

https://d2l.slack.com/archives/C0PHG3QB0/p1775842223156609

github-actions · 2026-04-14T14:44:38Z

Thanks for the PR! 🎉

We've deployed an automatic preview for this PR - you can see your changes here:

URL	https://live.d2l.dev/prs/BrightspaceUI/core/pr-6798/

Note

The build needs to finish before your changes are deployed.
Changes to the PR will automatically update the instance.

duncanuszkay-d2l · 2026-04-17T16:00:18Z

-
-		if (changedProperties.has('shown') && (
-			(reduceMotion && this._state === 'shown') || (!reduceMotion && this._state === 'showing')
-		)) {
-			this.#centerLoadingSpinner();
-		}


I replaced this check with a simpler one- rather than checking to see if we're in the first state after hidden, requiring us to check reduceMotion to see which that first state should be, instead we're just checking if the last state is hidden.

duncanuszkay-d2l · 2026-04-17T16:01:22Z

+const overlayStyles = css`
+	.action-slot::slotted(*) {
+		display: inline;


These styles are replicated from the simple empty state component

duncanuszkay-d2l · 2026-04-17T16:58:35Z

 			} else if (oldState === 'loading' && newState === 'clean') {
 				this.#setLiveArea(this.localize('components.backdrop-loading.loadingCompleteAnnouncement'));
+			} else if (newState === 'dirty') {
+				this.#setLiveArea(this.#renderDirtyOverlay(), { delay: DIRTY_ANNOUNCEMENT_DELAY });


To avoid complexities in coordinating the live region announcement and the display state of the visual overlay, I've opted to render the same content into the offscreen component. This results in an announcement that matches what you would get if the dirty overlay itself was in an aria-live component.

duncanuszkay-d2l · 2026-04-17T17:29:27Z

+			} else if (oldState === 'loading' && newState === 'clean') {
 				this.#setLiveArea(this.localize('components.backdrop-loading.loadingCompleteAnnouncement'));
+			} else if (newState === 'dirty') {
+				this.#setLiveArea(this.#renderDirtyOverlay(), { delay: DIRTY_ANNOUNCEMENT_DELAY });


I noticed some differences in screen reader behavior here that I feel are worth mentioning:

NVDA: Announces the text before the button, then the button text, with no pause. E.G. "Your data is out of date you should refresh your filters refresh"

Narrator: Communicates that there's a group with text and a button, E.G. "Group, Your data is out of date your should refresh your filters, button, refresh"

The latter I assume is more what we want, but ultimately the component is surfacing the necessary info to NVDA so I'm not sure if there's anything actionable here. Have we dealt with similar issues elsewhere in the Corel library before?

Updating vdiff goldens for PR 6798

duncanuszkay-d2l · 2026-04-22T16:44:46Z

@GZolla Ready for another look 👍 Thanks!

GZolla · 2026-04-22T17:20:22Z

+			},
 			/**
 			 * Used to identify content that the backdrop should make inert
 			 * @type {boolean}


Didn't notice this before

Suggested change

* @type {boolean}

* @type {string}

GZolla · 2026-04-22T17:43:40Z

 		}
 	}

+	willUpdate() {


I think this could use the PropertyRequiredMixin and a validator like in here.

GZolla · 2026-04-22T17:47:14Z

 			<div style="position:relative">
 				<slot id="table-slot" @slotchange="${this._handleSlotChange}"></slot>
-				<d2l-backdrop-loading for="table-slot" ?shown=${this.loading}></d2l-backdrop-loading>
+				<d2l-backdrop-loading @d2l-backdrop-dirty-overlay-action=${this._handleDirtyButton} for="table-slot" dataState=${this.dataState} dirty-text="${this.dirtyText}" dirty-button-text="${this.dirtyButtonText}"></d2l-backdrop-loading>


This should be a separate PR but I noticed that the scroll wrapper is not made inert meaning that it will still be focusable and scrollable, resulting in unexpected behavior. Maybe the backdrop should move to target the scroll wrapper instead

Hm, interesting- I'll dig deeper into that after this merges 👍

GZolla · 2026-04-22T17:50:25Z

+			}
+
+			d2l-backdrop-dirty-overlay {
+				background-color: var(--d2l-theme-backdrop-dialog-color);


Just confirming that it would no longer use a custom variable and the background would not match the table's controls background

would not match the table's controls background

I think that's fine, at least for now. I assume we could always override this value on the table side if we decide that's important.

GZolla

It's looking good, just missing the PropertyRequiredMixin for the validators to work

GZolla · 2026-04-22T18:21:21Z

+				reflect: true,
+				attribute: 'dirty-text',
+				required: {
+					validator: (_value, elem, hasValue) => {hasValue || elem.dataState !== 'dirty';}


Suggested change

validator: (_value, elem, hasValue) => {hasValue || elem.dataState !== 'dirty';}

validator: (_value, elem, hasValue) => hasValue || elem.dataState !== 'dirty'

GZolla · 2026-04-22T18:21:34Z

+				reflect: true,
+				attribute: 'dirty-button-text',
+				required: {
+					validator: (_value, elem, hasValue) => {hasValue || elem.dataState !== 'dirty';}


Suggested change

validator: (_value, elem, hasValue) => {hasValue || elem.dataState !== 'dirty';}

validator: (_value, elem, hasValue) => hasValue || elem.dataState !== 'dirty'

duncanuszkay-d2l changed the base branch from main to dunk.misc-table-changes April 14, 2026 14:43

This was referenced Apr 14, 2026

Updating translation formatting for PR 6798 #6799

Draft

Updating vdiff goldens for PR 6798 #6800

Merged

duncanuszkay-d2l force-pushed the dunk.dirty-dialog branch 3 times, most recently from 4cd674b to a373718 Compare April 16, 2026 17:43

duncanuszkay-d2l force-pushed the dunk.misc-table-changes branch from 033b515 to 00c1358 Compare April 16, 2026 18:27

duncanuszkay-d2l force-pushed the dunk.dirty-dialog branch from a373718 to 22897ac Compare April 16, 2026 18:58

Base automatically changed from dunk.misc-table-changes to main April 17, 2026 15:55

duncanuszkay-d2l force-pushed the dunk.dirty-dialog branch from 22897ac to 2dfcee7 Compare April 17, 2026 15:58

duncanuszkay-d2l commented Apr 17, 2026

View reviewed changes

duncanuszkay-d2l force-pushed the dunk.dirty-dialog branch from 2dfcee7 to 24808ad Compare April 17, 2026 16:56

duncanuszkay-d2l commented Apr 17, 2026

View reviewed changes

duncanuszkay-d2l force-pushed the dunk.dirty-dialog branch 5 times, most recently from 54c986a to 7903c49 Compare April 17, 2026 19:11

duncanuszkay-d2l requested review from GZolla April 17, 2026 19:47

duncanuszkay-d2l marked this pull request as ready for review April 17, 2026 19:48

duncanuszkay-d2l requested a review from a team as a code owner April 17, 2026 19:48

Duncan Uszkay added 2 commits April 22, 2026 11:17

Move to multi-state prop

440e6be

Add dirty state dialog

d345775

duncanuszkay-d2l force-pushed the dunk.dirty-dialog branch from e3d0884 to d345775 Compare April 22, 2026 15:17

Updating vdiff goldens

4c63ef0

github-actions Bot mentioned this pull request Apr 22, 2026

Updating vdiff goldens for PR 6798 #6843

Merged

Merge pull request #6843 from BrightspaceUI/ghworkflow/vdiff-pr-6798

0583e15

Updating vdiff goldens for PR 6798

GZolla reviewed Apr 22, 2026

View reviewed changes

Comment thread components/backdrop/backdrop-dirty-overlay.js Outdated

Comment thread components/backdrop/backdrop-loading.js Outdated

Comment thread components/backdrop/backdrop-loading.js Outdated

PR Feedback

154f31b

GZolla reviewed Apr 22, 2026

View reviewed changes

PR Feedback

30496cb

GZolla reviewed Apr 22, 2026

View reviewed changes

	validator: (_value, elem, hasValue) => {hasValue \|\| elem.dataState !== 'dirty';}
	validator: (_value, elem, hasValue) => hasValue \|\| elem.dataState !== 'dirty'

Conversation

duncanuszkay-d2l commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Demo

Uh oh!

github-actions Bot commented Apr 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

duncanuszkay-d2l commented Apr 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GZolla left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

duncanuszkay-d2l commented Apr 14, 2026 •

edited

Loading