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

[SolutionNav] Fixed tabindex and collapsible functionality #107462

Merged
merged 7 commits into from Aug 9, 2021

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Aug 2, 2021

Fixed tabindex

The Solution Nav component was setting tabindex=-1 manually on every side nav item passed in when the side nav was set to hidden because of the collapsibility. Unfortunately, it was not removing this when the side nav was made visible again, degrading the keyboard accessibility from then on.

I've changed that function to accept the isHidden prop instead and then set the tabindex based on that prop.

item.tabIndex = isHidden ? -1 : undefined;

Increased usability of the collapsible button

Gave the button a background color when in expanded mode.

Before, the button could potentially overlap the scrollbar making it hard to distinguish. I simply changed this to have a white (empty shade) background so that it stays visible with still being subtle.

image

And added the embellishment back in 😉

Made the collapsed version of the button expand the entire collapsed area

Before, the target area of the expand button was only 24x24px which is really small especially on medium sized screens when this is the only way to trigger the solution nav. I have instead grown the button to expand the entire area creating a large hit area for expansion. (And also decreased the width to give more space back to the content).

image

Checklist

Delete any items that are not applicable to this PR.

@cchaos
Copy link
Contributor Author

cchaos commented Aug 3, 2021

@elasticmachine merge upstream

Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

code owner change LGMT

@cchaos
Copy link
Contributor Author

cchaos commented Aug 9, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kibanaReact 150.6KB 152.7KB +2.1KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@cchaos cchaos merged commit be7ec79 into elastic:master Aug 9, 2021
@cchaos cchaos added auto-backport Deprecated: Automatically backport this PR after it's merged and removed auto-backport Deprecated: Automatically backport this PR after it's merged labels Aug 9, 2021
cchaos added a commit to cchaos/kibana that referenced this pull request Aug 9, 2021
…#107462)

* Fixed `tabIndex`
* Added embellishment back in
* Fixed the collapse button visibility and usability
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 9, 2021
…#107462)

* Fixed `tabIndex`
* Added embellishment back in
* Fixed the collapse button visibility and usability
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

@cchaos cchaos deleted the fix/solution_nav branch August 9, 2021 17:40
cchaos added a commit that referenced this pull request Aug 9, 2021
#107943)

* Fixed `tabIndex`
* Added embellishment back in
* Fixed the collapse button visibility and usability
@KOTungseth KOTungseth added the Feature:Home Kibana home application label Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants