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

[core] fix(Overlay): capture focus when it returns to document #4907

Merged
merged 3 commits into from
Sep 20, 2021

Conversation

michael-yx-wu
Copy link
Contributor

@michael-yx-wu michael-yx-wu commented Sep 14, 2021

Follow up to #4894 (comment)

Screenshot

Before -- focus would not return to overlay if coming back to the document from somewhere outside (e.g. url, native browser controls)

2021-09-14 17 19 46

After -- focus immediately returns to overlay

2021-09-14 17 21 39

const decoratedChild =
typeof child === "object" ? (
React.cloneElement(child as React.ReactElement, {
className: classNames((child as React.ReactElement).props.className, Classes.OVERLAY_CONTENT),
tabIndex: this.props.enforceFocus || this.props.autoFocus ? 0 : undefined,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing this fixes an issue w/ the omnibar where the container and the input are both focusable, even though you probably don't want the container to be focusable in that situation. now that we have focus traps, this doesn't seem necessary. if consumers want non-focusable elements to be focusable, they will need to take care to set tabindex themselves. not sure if this constitutes a break

Base automatically changed from mw/dialog-enforceFocus to develop September 14, 2021 21:52
@michael-yx-wu michael-yx-wu force-pushed the mw/dialog-enforceFocus-from-browser branch from b770f77 to 2bc9d82 Compare September 15, 2021 02:42
@blueprint-bot
Copy link

Capture focus when it returns to the document

Previews: documentation | landing | table

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

code is a little convoluted, but seems ok...

there is a bug now with dialog focus... seems a little too aggressive. you can't focus the primary button or the close button via the TAB key:

2021-09-15 11 54 01

@michael-yx-wu
Copy link
Contributor Author

michael-yx-wu commented Sep 16, 2021

I think this is a result of the infinite focus bug due to shouldReturnFocusOnClose that was fixed in 49a2a46. Merged in develop and it works properly now:
2021-09-16 15 23 52

@blueprint-bot
Copy link

Comment grammar

Previews: documentation | landing | table

@adidahiya adidahiya changed the title Capture focus when it returns to the document [core] fix(Overlay): capture focus when it returns to document Sep 20, 2021
@adidahiya adidahiya merged commit 32b7e47 into develop Sep 20, 2021
@adidahiya adidahiya deleted the mw/dialog-enforceFocus-from-browser branch September 20, 2021 20:58
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.

3 participants