Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
duncanuszkay-d2l
force-pushed
the
dunk.apply-button
branch
from
d74670b to
5ef9e55
Compare
duncanuszkay-d2l
force-pushed
the
dunk.apply-button
branch
from
908ba31 to
ca89028
Compare
duncanuszkay-d2l
force-pushed
the
dunk.apply-button
branch
from
ca89028 to
1703061
Compare
| if (changedProperties.has('shown') && ( | ||
| (reduceMotion && this._state === 'shown') || (!reduceMotion && this._state === 'showing') | ||
| )) { | ||
| this.#centerLoadingSpinner(); | ||
| } |
There was a problem hiding this comment.
This centering behavior is re-addressed in a later commit
| } | ||
| :host([_state="showing"]), | ||
| :host([_state="shown"]), | ||
| :host([_state="loading"]), |
There was a problem hiding this comment.
The shown state now represents the dirty dialog being open:
The loading state represents the loading spinner being visible:
| if (this._state === 'hidden') return nothing; | ||
| return html` | ||
| <div class="backdrop" @transitionend="${this.#handleTransitionEnd}" @transitioncancel="${this.#hide}"></div> | ||
| <div class="backdrop" @transitionend="${this.#handleTransitionEnd}" @transitioncancel="${this.#handleTransitionEnd}"></div> |
There was a problem hiding this comment.
This resolved a bug I began to run into once I adjusted the demo to allow the more rapid state changes- we can't assume that a cancellation of a transition must indicate that we need to hide, we need to check to make sure that we're in the hiding state.
| setTimeout(() => { | ||
| if (this._state === 'showing') this._state = 'shown'; | ||
| }, BACKDROP_DELAY_MS); |
There was a problem hiding this comment.
We've decided to remove this delay and instead show the backdrop as soon as the data becomes dirty
| if (changedProperties.has('dataState') && | ||
| changedProperties.get('dataState') === 'clean' && | ||
| ( | ||
| (reduceMotion && this._state === 'shown') || | ||
| (!reduceMotion && this._state === 'showing') || | ||
| (!reduceMotion && this._state === 'loading') | ||
| ) | ||
| ) { |
There was a problem hiding this comment.
We only want to center if we're moving from a hidden/clean state into a non-hidden state, we don't want to center again when moving from the dialog to the loading spinner
| const lowerVisibleBound = Math.min(window.innerHeight, boundingRect.bottom); | ||
| const visibleHeight = lowerVisibleBound - upperVisibleBound; | ||
| const centeringOffset = visibleHeight / 2; | ||
| const centeringOffset = (visibleHeight / 4); |
There was a problem hiding this comment.
From design feedback on the demo, we're moving to a 25% offset instead of centering (50%).
| await this.shadowRoot.querySelector('d2l-empty-state-simple').getUpdateComplete(); | ||
| await this.shadowRoot.querySelector('d2l-empty-state-action-button').getUpdateComplete(); |
There was a problem hiding this comment.
This ensures that we're accurately measuring the height offset for the dialog box, this should remain consistent regardless of the text content
| this._spinnerTop = centeringOffset + topOffset - spinnerSizeOffset; | ||
| this._dirtyDialogTop = centeringOffset + topOffset - dirtyDialogSizeOffset; |
There was a problem hiding this comment.
I'm removing the minimum buffer, when the table get's small it's fine to overlay the headers as per design's demo feedback
duncanuszkay-d2l
force-pushed
the
dunk.apply-button
branch
from
ea2f025 to
6c65b4d
Compare
duncanuszkay-d2l
force-pushed
the
dunk.apply-button
branch
from
6c65b4d to
1d1bf11
Compare
1 participant
Note for reviewers- looking for some early input on direction before finishing this up 👀
https://desire2learn.atlassian.net/browse/NTNGL-5471?atlOrigin=eyJpIjoiMDk2MmRlZjZmMjkxNGNhZjlmMTIxMTdiMjJiODZjODYiLCJwIjoiaiJ9
Functional Testing
Moving between the new backdrop states:
Recording.2026-04-07.163437.mp4
Using the new system in the downstream consumer (WIP integration PR)
Recording.2026-04-07.163602.mp4