Update d2l-alert-toast to use popover - #7338
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. |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
All the visual-diffs are tiny pixel diffs on some corners, and overlapping shadows. Seems like it might be something different with how these are rendered on the top layer... I think the position should be identical. |
| right: 0; | ||
| width: 100%; | ||
| } | ||
| .d2l-alert-toast-container-not-popover { |
There was a problem hiding this comment.
Wondering if we might want to have 2 copies of all (or most) of the alert-toast vdiffs, one for popover on and one for off?
Reasoning:
- There should be no vdiffs for the OFF case with this PR
- We'd catch any future regressions to the OFF case
Of course having double the vdiffs isn't amazing, but it would just be temporary for the next year or two until popover is supported everywhere.
| if (newOpen) { | ||
|
|
||
| // Clean-up when removing GAUD-10337-use-alert-toast-popover | ||
| if (alertToastPopoverFlag) { |
There was a problem hiding this comment.
Shouldn't these two ifs be if (usePopover) { and not just the flag? (then there's no cleaning them up with the flag, but only when popover is supported everywhere.
2 participants
GAUD-10337
This PR updates d2l-alert-toast to use native popover if supported.