fix(tabs): content animation in RTL not working (chrome) #12215
Conversation
Fixes that the tabs content is not animating properly in chrome when RTL is turned on. Fixes angular#8833. Fixes angular#9465
devversion
added
P2
googlebot
added
the
cla: yes
devversion
removed
the
Accessibility
| // in order to ensure that the element has a height before its state changes. This is | ||
| // necessary because Chrome does seem to skip the transition in RTL mode if the element does | ||
| // not have a static height and is not rendered. See related issue: #9465 | ||
| state('left', style({transform: 'translate3d(-100%, 0, 0)', minHeight: '1px'})), |
There was a problem hiding this comment.
Can we do this directly in the stylesheet? I think we have a class for the off-screen tabs which should allow us to set the min-height.
There was a problem hiding this comment.
We only have a class for the active tab body, so we could work with :not, but since this technically belongs to the animation setup I'd anyway prefer having this here.
That way if someone looks for the animations, he can quickly see that there is some special workaround applied.
There was a problem hiding this comment.
True, but this makes it a lot more difficult for somebody to change the min-height, if they wanted to.
There was a problem hiding this comment.
That min-height is only temporarily while the tab is hidden. As soon as it transitions to the visible state, the min-height is completely gone.
Also, why would someone need to update the temporary min-height of an internal and hidden element? The developer can still set his own min-height in the projected content easily.
I'm fine changing if you really want to. Just think if we move into some SCSS file, we'd need some special selector (e.g. :not), and at some point we would not remember about that specific workaround that belongs to the animation.
There was a problem hiding this comment.
The way I'm viewing it is that it's unlikely for somebody to want to change the min-height, but if they do, it would be very hard for them to do so, whereas it's very low-effort for us to have it in the stylesheet to begin with. Having it there also ends up being less generated code.
There was a problem hiding this comment.
I see your point with the low-effort to place in the stylesheet. In regards to the code size, I think the generated CSS will take up more space than how it's right now.
Also, if we consider it like that, we could also argue about moving the actual transform's that don't include any computed values to the stylesheet.
Anyway, I will switch it to the CSS but keep a comment that mentions the Chrome workaround that is placed in the stylesheet.
There was a problem hiding this comment.
As discussed on Slack; since depending on the .mat-tab-body-active does not work because the class is being added before hidden tabs start the actual animation, we are keeping the workaround in the TS file.
crisbeto
added
pr: lgtm
action: merge
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes that the tabs content is not animating properly in chrome when RTL is turned on.
Fixes #8833. Fixes #9465