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.

Sign up for GitHub

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

Jump to bottom

[Metrics UI] Tweak the z-index inventory toolbars #49642

Merged
merged 1 commit into from
Oct 30, 2019
Merged

[Metrics UI] Tweak the z-index inventory toolbars #49642

merged 1 commit into from
Oct 30, 2019

Conversation

afgomez
Copy link
Contributor

@afgomez afgomez commented Oct 29, 2019 •
edited
Loading

Fixes #49601.

Adds a zIndexDiff attribute to the <Toolbar> component that allows the consumer to individually tweak the z-index of each individual toolbar. By lowering the z-index attribute of the second toolbar, we prevent it from covering the dropdowns of the first one

Removes the z-index of the <Toolbar> to prevent stacking issues with the dropdowns of the different toolbars when there are two or more together.

Screenshot 2019-10-29 at 16 01 05

@afgomez afgomez added release_note:fix Feature:Metrics UI Metrics UI feature v8.0.0 Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.5.0 labels Oct 29, 2019
@afgomez afgomez requested a review from a team as a code owner October 29, 2019 15:05
@elasticmachine
Copy link
Contributor

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

@phillipb
Copy link
Contributor

This makes sense. I'm kind of confused about why any of these toolbars have a z-index. Seems that they're unnecessary. What happens if you just remove it?

@afgomez
Copy link
Contributor Author

afgomez commented Oct 29, 2019 •
edited
Loading

@phillipb there are elements below the toolbars that have high z-index values. If we remove the z-index those elements will be higher in the stack.

Screenshot 2019-10-29 at 16 31 43

Edit: funnily enough the high z-index doesn't fix the problem in the screenshot :/

@phillipb
Copy link
Contributor

phillipb commented Oct 29, 2019 •
edited
Loading

@afgomez is it just the tooltip that does that? If that's the case, IMO, that might be ok since it's a fringe scenario. Also, seems like we could just up the z-index of the popover menus since they already have one.

Basically, just trying to avoid the extra cognitive overhead/maintenance of an extra prop here.

@simianhacker might have some thoughts here too.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Remove z-index from the Metrics <Toolbar>
4ce76f5 
Having a z-index adds problems with the stacking of the dropdowns when
there are two toolbars together.
@afgomez afgomez force-pushed the 49601-z-index-metrics-toolbar branch from 645dee2 to 4ce76f5 Compare October 29, 2019 16:28
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@afgomez
Copy link
Contributor Author

afgomez commented Oct 29, 2019

Tested in all browsers (included IE11) without z-index and everything looks correct.

Screenshot 2019-10-29 at 22 24 06

@phillipb phillipb self-assigned this Oct 29, 2019
phillipb
phillipb approved these changes Oct 29, 2019
View reviewed changes
Copy link
Contributor

@phillipb phillipb left a comment

Choose a reason for hiding this comment

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

LGTM!

@afgomez afgomez merged commit d955797 into elastic:master Oct 30, 2019
@afgomez afgomez deleted the 49601-z-index-metrics-toolbar branch October 30, 2019 08:26
This was referenced Oct 30, 2019
Closed
Merged
afgomez pushed a commit to afgomez/kibana that referenced this pull request Oct 30, 2019
[Metrics UI] Tweak the z-index inventory toolbars (elastic#49642)
1133146 
Having a z-index adds problems with the stacking of the dropdowns when
there are two toolbars together.
afgomez pushed a commit to afgomez/kibana that referenced this pull request Oct 30, 2019
[Metrics UI] Tweak the z-index inventory toolbars (elastic#49642)
a1afd1e 
Having a z-index adds problems with the stacking of the dropdowns when
there are two toolbars together.
afgomez pushed a commit to afgomez/kibana that referenced this pull request Oct 30, 2019
[Metrics UI] Tweak the z-index inventory toolbars (elastic#49642)
8db89a5 
Having a z-index adds problems with the stacking of the dropdowns when
there are two toolbars together.
@afgomez afgomez mentioned this pull request Oct 30, 2019
Merged
This was referenced Nov 18, 2019
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phillipb phillipb phillipb approved these changes

Assignees

@phillipb phillipb

Labels
Feature:Metrics UI Metrics UI feature release_note:fix Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.5.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Metrics UI] Adjust z-index of the filtering bar
3 participants
@afgomez @elasticmachine @phillipb