Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dialog): text is now selectable #5347

Merged
merged 1 commit into from
Dec 27, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions dialog/internal/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,6 @@ export class Dialog extends LitElement {
requestUpdateOnAriaChange(Dialog);
}

/** @nocollapse */
static override shadowRootOptions = {
...LitElement.shadowRootOptions,
delegatesFocus: true,
};

/**
* Opens the dialog when set to `true` and closes it when set to `false`.
*/
Expand Down Expand Up @@ -133,6 +127,26 @@ export class Dialog extends LitElement {
super();
if (!isServer) {
this.addEventListener('submit', this.handleSubmit);

// We do not use `delegatesFocus: true` due to a Chromium bug with
// selecting text.
// See https://bugs.chromium.org/p/chromium/issues/detail?id=950357
//
// Material requires using focus trapping within the dialog (see
// b/314840853 for the bug to add it). This would normally mean we don't
// care about delegating focus since the `<dialog>` never receives it.
// However, we still need to handle situations when a user has not
// provided an focusable child in the content. When that happens, the
// `<dialog>` itself is focused.
//
// Listen to focus/blur instead of focusin/focusout since those can bubble
// from content.
this.addEventListener('focus', () => {
this.dialog?.focus();
});
this.addEventListener('blur', () => {
this.dialog?.blur();
});
}
}

Expand Down
Loading