fix(PopupMenu): add prop popupMenuDropdownZIndex

[Unic0arn]

Allow specifying z-index on the dropdown from the popupMenu

[coveralls]

Coverage remained the same at 83.811% when pulling 19665c5f0b98def1135d18c3eee8ff6b96605061 on add-zIndex-prop-to-popupMenuDropdown into 10a7b696a7cd539340fde7de2a0021992ba3b673 on master.

[coveralls]

Coverage remained the same at 83.811% when pulling 19665c5f0b98def1135d18c3eee8ff6b96605061 on add-zIndex-prop-to-popupMenuDropdown into 10a7b696a7cd539340fde7de2a0021992ba3b673 on master.

[JacobBlomgren]

What's the reason behind this?

[Unic0arn]

What's the reason behind this?

Sticky headers on tables also have z-index 1 so the popup renders behind the table header if that's enabled.

[JacobBlomgren]

Sticky headers on tables also have z-index 1 so the popup renders behind the table header if that's enabled.

Ok. Could we just change the z-index here instead of passing it in as a prop? That feels like a solution that will not mature nicely, I'm afraid.

[Unic0arn]

Sticky headers on tables also have z-index 1 so the popup renders behind the table header if that's enabled.

Ok. Could we just change the z-index here instead of passing it in as a prop? That feels like a solution that will not mature nicely, I'm afraid.

Why will it not mature nicely? I prefer to have it default to 1 and then if the need arises we have the possibility to override it. Also, adding this as a prop won't change current behaviour if someone relies on it being 1.

[JacobBlomgren]

Why will it not mature nicely? I prefer to have it default to 1 and then if the need arises we have the possibility to override it. Also, adding this as a prop won't change current behaviour if someone relies on it being 1.

Because I feel the problem we have is that we go about changing our components the wrong way, by customizing rather than considering if the component everywhere should be updated. Which makes all updates harder.

If we allow this as a prop, there will be several hard to find places in the wild with different z-indeces. It's better to contain it to as few places as possible. And also go over our current usage of them because I know that this component clashes with the mobile navbar on our site too.

[Unic0arn]

Why will it not mature nicely? I prefer to have it default to 1 and then if the need arises we have the possibility to override it. Also, adding this as a prop won't change current behaviour if someone relies on it being 1.

Because I feel the problem we have is that we go about changing our components the wrong way, by customizing rather than considering if the component everywhere should be updated. Which makes all updates harder.

If we allow this as a prop, there will be several hard to find places in the wild with different z-indeces. It's better to contain it to as few places as possible. And also go over our current usage of them because I know that this component clashes with the mobile navbar on our site too.

I disagree. z-indices are hard enough to deal with without having arbitrary numbers specified deep in sub components. I think having default values of 1 like here and then in the special case where it's needed overwrite it is better.

Bumping it to 2 here won't solve the bottom nav issue. That issue should be solved by looking at the stacking contexts of the menus.

[JacobBlomgren]

I disagree. z-indices are hard enough to deal with without having arbitrary numbers specified deep in sub components. I think having default values of 1 like here and then in the special case where it's needed overwrite it is better.

Bumping it to 2 here won't solve the bottom nav issue. That issue should be solved by looking at the stacking contexts of the menus.

No it doesn't solve it, but neither does allowing the possibility for even more spread out arbitrary numbers, imo.