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

[EuiFlyout] Improve usability of Kibana global search when flyout is open #5206

Closed
peteharverson opened this issue Sep 21, 2021 · 9 comments · Fixed by #6566
Closed

[EuiFlyout] Improve usability of Kibana global search when flyout is open #5206

peteharverson opened this issue Sep 21, 2021 · 9 comments · Fixed by #6566
Labels
⚠️ needs spec Should be groomed by the EUI team every week to ensure a spec is added

Comments

@peteharverson
Copy link

With an EuiFlyout open, it is not possible to use the input in the Kibana global search bar. As soon as you click in the search input to give it focus, focus is transferred back to the flyout.

flyout_global_search

It would be ideal if the global search bar could retain focus, or if this is not possible provide better usability to prevent the user trying to interact with elements outside of the flyout.

@chandlerprall chandlerprall added assign:engineer ⚠️ needs spec Should be groomed by the EUI team every week to ensure a spec is added labels Sep 21, 2021
@chandlerprall
Copy link
Contributor

We'll need to figure out how we want to resolve these UX focus-fighting issues, then determine the technical approach to solving. To complicate things further, this probably involves allowing some kind of configuration by applications - e.g. maybe setting traps' importance/priority.

I doubt we'll have the ability to take this on effort anytime soon, but it's certainly a very valid UX issue to resolve.

@rahulbhadja
Copy link

Hey Chandler, i'm trying to fix both issues in EuiFlyout. Anyone is working on the same?

@cee-chen
Copy link
Contributor

@peteharverson - is this still an issue for your team in Kibana? @ryankeairns did a bit of testing with latest main and it looks like the global search + flyout flicker happens in Stack Mgmt and Integrations, but works in other areas like Obs rule creation, Discover, etc.

I believe what you need to do here is use the focusTrapProps.shards API of EuiFlyout to pass the entire sticky nav bar element to, so that our focus trap considers it part of the flyout and does not attempt to restore focus on click.

See #5860 for more details

@cee-chen cee-chen added answered Issues answered by the EUI team - auto-closes open issues in 7 days if no follow-up response and removed skip-stale-check labels Jan 30, 2023
@peteharverson
Copy link
Author

@cee-chen yes we still need to resolve this for the flyouts inside ML, and as @ryankeairns reported it still affects other pages such as Index Management under Stack Management.

Do you have some sample code which shows how we can pass the nav bar element into focusTrapProps.shards, or could you point me to how this is working on other pages?

@github-actions github-actions bot removed the answered Issues answered by the EUI team - auto-closes open issues in 7 days if no follow-up response label Jan 31, 2023
@cee-chen
Copy link
Contributor

cee-chen commented Jan 31, 2023

Did a bit more digging, and hilariously enough... You can actually see the exact focus issue you reported happening on our "Elastic navigation pattern" demo 😅 Which is actually a great opportunity for me to exemplify how to fix the problem:

https://github.com/elastic/eui/pull/6566/files

Also, just thinking out loud - after poking around a bit more at our various header/page template patterns, I wonder if it would make sense for EUI to just automatically bake all fixed EuiHeaders on the page into every EuiFlyout's focusTrapProps.shards, to skip every single team in Kibana having to do that.

I'm a little worried there's some edge case I haven't thought of where EuiFlyout wouldn't sit under the fixed header, but I can't think of one? We could probably provide some kind of includeHeadersInFocusTrap flag as an escape hatch for consumers who don't want that behavior, I guess.

Thoughts and opinions very much appreciated!

@cee-chen
Copy link
Contributor

After poking at this a bit more locally, I think we need to implement this on EUI's side after all, mostly due to accessibility concerns. There's a lot of focus/accessibility things that start happening once you include headers in focusTrapProps.shards. The headers becoming part of the keyboard tab rotation is the biggest one - which technically should be the case since it was already happening for mouse users, but I need to work with our resident a11y expert @1Copenut to make sure I'm doing this correctly.

Trevor, do you have some time Wednesday morning to hop on a screenshare to brainstorm default flyout focus trap behavior?

@peteharverson
Copy link
Author

Thanks for posting updates @cee-chen. Agree it sounds best for the behavior here to be addressed on the EUI side if possible.

@cee-chen
Copy link
Contributor

cee-chen commented Feb 6, 2023

@peteharverson Just as a heads up, this fix will not be in by 8.7 unfortunately, but will be in for 8.8. Hoping that's okay given the age of this issue, but please feel free to let us know if not.

@peteharverson
Copy link
Author

@cee-chen 8.8 is fine by me. Glad you managed to find a fix on the EUI side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ needs spec Should be groomed by the EUI team every week to ensure a spec is added
Projects
None yet
4 participants