Automatically set Tooltip z-index according to the parent's z-index #1221

Closed
wants to merge 1 commit into
from

Projects

None yet

7 participants

@pokonski
Contributor

I went ahead and made some changes to fix the z-indexes as described in #1109.
This code automatically sets increased z-index of its parent (if any parent has z-index set at all) to make Tooltips and Popovers visible in .modal.

I didn't want to restrict it to a class name so I filter parents by finding the closest one with that CSS attribute set. This way it will work not only in modals but with other elements users might create.

Works fine on Opera/Chrome/Firefox. The only concern might be performance, but in Bootstrap docs even in the worst scenario (Tooltips are not in modals, therefore filter checks parents up to <html>) this is only 8 elements to check => no visible slow-downs.

@fat, can you take a look? Demo => http://okonski.org/bootstrap-zindex/docs/javascript.html#modals

EDIT: Seems to work just fine for fixed navbars, too! :)

@addisonhardy

I am also not seeing popovers/tooltips inside the modal window as coded here: http://markdotto.com/bs2/docs/javascript.html#modals.

Chrome / Mac OSX.

@pokonski your patch fixes this issue for me.

@mdo
Member
mdo commented Jan 26, 2012

@pokonski: Siiiiiick, bro. If @fat gives the go ahead, we'll get this merged in ASAP. <3

@XORwell
XORwell commented Jan 26, 2012

awesome! hope it get merged soon. thx alot! :)

@fat
Member
fat commented Jan 28, 2012

lol - that is a pretty funny hack - but we can't actually do this unfortunately. We definitely need to think about other websites (not just our docs), where performance would really matter.

I think adding the z-index option is reasonable though... maybe at most checking to see if you're element is within the modal.

@fat fat closed this Jan 28, 2012
@pokonski
Contributor

Well then, a bootstrap-modal.js could set the data-z-index attribute for all elements inside that trigger tooltips so complexity is much smaller, no traversing upwards at all.

And checking if element is within the modal is the same thing, closest(".modal") goes up until it finds a .modal which in case of elements outside of modals will go as high as <html>.

@fat
Member
fat commented Jan 28, 2012

Yeah - i opted for just giving a z-index option. I don't want to try to do any magic with this.

@pokonski
Contributor

Will you mention that in the docs? Otherwise people will keep submitting issues.

The performance aspect got me interested, will benchmark the traversing upwards more intensively. Because except that there are no downsides to finding out parent's z-index.

@fat
Member
fat commented Jan 28, 2012

The other downside is this code looks awful and I don't want it in our source :)

tp.zIndex = this.options.zIndex || parseInt(this.$element.parents().filter(function() {
   return $(this).css('z-index') != 'auto';
}).first().css('z-index'))+10

Beyond that, if you think about it, it doesn't really make sense either.

You're getting an array of z-indexes and taking the first one and adding 10 to it.

That means if i have a grandparent with a z-index of 1000 and parent with 100 and then my element. My element will only get a z-index of 110 - which will still show beneath the grandparent.

This isn't an impossible problem to solve this way - it's just one i'm unwilling to do because it's janky.

I'm happy to add a note about this in the docs.

@pokonski
Contributor

It's not pretty, I agree. I was hoping to see you work your magic and
making it look nice :D

Well, if parent has a z-index smaller than grandparent isn't that a
mistake in styles to begin with?

@fat
Member
fat commented Jan 28, 2012

actually i have a way better solution... give me 15 min...

@fat
Member
fat commented Jan 28, 2012

So this is the solution I think we'll roll with: 12d3c2f

The reason i like this so much is because I really want to try to keep all style stuff in the CSS - that means not setting any styles on elements like z-index if we can avoid it.

@pokonski
Contributor

Wow, so simple it's hard to believe no one came up with that before :D

@fat
Member
fat commented Jan 28, 2012

:)

@mr-stateradio

Which is the first bootstrap version that included this fix? For me it looks like the posted solution is in the branch for 2.3.0, which will be published soon. Is this correct? I'm using v.2.2.2 and I'm facing the same issue that the popup is added to the dom but is not visible. Whereas if this fix is already included in 2.2.2. my issue has probably a different source.

@tsuna
tsuna commented Jun 23, 2013

I'm still seeing the problem in v2.3.2

@tsuna
tsuna commented Jun 23, 2013

ah sorry it looks like I'm a victim of #6344

@cvrebert cvrebert locked and limited conversation to collaborators Jul 28, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.