Skip to content

Update d2l-dialog to prefer native dialog rendering. - #7446

Open
dbatiste wants to merge 6 commits into
mainfrom
dbatiste/dialog-prefer-native
Open

Update d2l-dialog to prefer native dialog rendering.#7446
dbatiste wants to merge 6 commits into
mainfrom
dbatiste/dialog-prefer-native

Conversation

@dbatiste

@dbatiste dbatiste commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

GAUD-10409

This PR updates d2l-dialog to prefer native dialog element.

Like the previous PRs that similarly updated d2l-dialog-fullscreen and d2l-dialog-confirm, this includes a change in focus behaviour. Specifically, the header is focused. (much investigation was done by Margaree #6760). Another focus behaviour difference is due to not using d2l-focus-trap, as explained in #2424. d2l-dialog-confirm adopted that change as well, though I am not sure if we noticed.

@dbatiste
dbatiste requested a review from a team as a code owner September 4, 2026 14:37
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

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-7446/

Note

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

<!DOCTYPE html>
<html lang="en">
<head>
<script>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was not working as originally developed since previous changes were made. I've removed the double tests for the ifrau case since I don't think they are necessary for both native and custom impls.

before(() => mockFlag(preferNativeGeneralDialogsFlag, type === 'native'));
after(() => resetFlag(preferNativeGeneralDialogsFlag));

const focusOnContentTestCases = (type === 'custom') ? [

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These test cases are only applicable in custom mode, so I've pulled them out to only run when custom.

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.

1 participant