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

Reduce z-indexes in button-group, input-group, list-group, and pagination to the minimum necessary #24315

Merged
merged 3 commits into from
Oct 20, 2017

Conversation

rmacklin
Copy link
Contributor

@rmacklin rmacklin commented Oct 9, 2017

These were using z-index: 2 to "Place active items above their siblings for proper border styling". However, using z-index: 1 is sufficient for accomplishing that goal.

In input-group, there were also three z-index: 3 rules for the hover/focus/active states. I reduced these to z-index: 2 since they just needed to be "one more than normal" (i.e. one more than what is now z-index: 1 after my changes).

These changes can be verified by viewing the documentation pages for Button group, Input group, List group, and Pagination before and after this commit and observing that the active elements are still "above" their siblings, so their borders look correct. (In fact, the easiest way may just be to change these z-indexes in the dev tools style editor - that's what I did before opening this PR.)

I also added a brief paragraph in the documentation about these z-indexes.

These were using `z-index: 2` to "Place active items above their
siblings for proper border styling". However, using `z-index: 1` is
sufficient for accomplishing that goal.

In input-group, there were also three `z-index: 3` rules for the
hover/focus/active states. I reduced these to `z-index: 2` since they
just needed to be "one more than normal" (i.e. one more than what is now
`z-index: 1` after my changes).

These changes can be verified by viewing the documentation pages for
Button group, Input group, List group, and Pagination before and after
this commit and observing that the active elements are still "above"
their siblings, so their borders look correct.
to include the z-index settings for button-group, input-group,
list-group, and pagination
Copy link
Collaborator

@andresgalante andresgalante left a comment

Choose a reason for hiding this comment

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

@rmacklin Thanks for the PR, you are right there is no need to use 2 and 3 z-indexes.

I don't know if @mdo did it intentionally to leave space for a possible z-index: 1 element in the future.

I also don't think there is a need to document this. I don't see the relationship between the z-index of these elements and layouts.

@mdo mdo added this to Inbox in v4 Beta 3 Oct 19, 2017
@mdo mdo moved this from Inbox to Ready to merge in v4 Beta 3 Oct 20, 2017
@XhmikosR XhmikosR merged commit 9e6dabb into twbs:v4-dev Oct 20, 2017
@mdo mdo mentioned this pull request Oct 20, 2017
@rmacklin
Copy link
Contributor Author

Thanks!!

@rmacklin rmacklin deleted the reduce-z-indexes-to-minimum-necessary branch October 20, 2017 16:43
@mdo mdo moved this from Ready to merge to Shipped in v4 Beta 3 Oct 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
v4 Beta 3
  
Shipped
Development

Successfully merging this pull request may close these issues.

None yet

4 participants