Skip to content
This repository was archived by the owner on Jan 22, 2026. It is now read-only.

Popover fix#61

Merged
sarahzinger merged 7 commits intomasterfrom
popover-fix
Nov 5, 2019
Merged

Popover fix#61
sarahzinger merged 7 commits intomasterfrom
popover-fix

Conversation

@sarahzinger
Copy link
Copy Markdown
Contributor

@sarahzinger sarahzinger commented Nov 1, 2019

Currently when you open a popover and click anywhere else, your focus returns back to the popover button that opened it. This was done purposely to ensure users do not lose focus when they press escape. But it looks like we've accidentally created other focus bugs by accident. Now if you click to open a popover and then click on any other interactive element your focus does not go to that interactive element but back to the old popover to show you closed it. This means if you're clicking around between popovers or between input fields it's pretty easy to get confused. Here's a screen recording illustrating what I'm talking about: https://recordit.co/0e7KhdW4Mf

My first thought was to remove this functionality entirely, but I realized when you tab into a popover and then press escape you lose your focused element. So now I'm suggesting that in general we don't steal back focus except if you use escape to close in which case grab back focus to the button that opens the popover.

To play around with this you can use:
https://fluff-hollyhock.glitch.me/#StoryPopover

or you can use
https://glass-mercury.glitch.me/
which should have the changed Popover component running, it might be helpful to see the git diff here:
https://github.com/FogCreek/Glitch-Community/pull/1046/files
I tried to mock out all the Popovers in that remix, but I probably missed a few, irl we won't need to change the community code at all.

Shoutout to Cassey (and Greg!) for pairing on this!

@sarahzinger sarahzinger requested a review from clottman November 1, 2019 20:59
Copy link
Copy Markdown
Contributor

@clottman clottman left a comment

Choose a reason for hiding this comment

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

👍

@sarahzinger sarahzinger merged commit 09dfdf5 into master Nov 5, 2019
@sarahzinger sarahzinger deleted the popover-fix branch November 5, 2019 15:19
@keithk
Copy link
Copy Markdown
Contributor

keithk commented Nov 10, 2020

🚀 PR was released in v0.19.0 🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants