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

fix: avoid layout jumping on elements with ripples in RTL #10026

Merged
merged 1 commit into from
Feb 26, 2018

Conversation

crisbeto
Copy link
Member

A while ago we added a backface-visibility to some components that have scrollable content in order to avoid repaints while scrolling (see #7889, #7721, #7719, #6890, #2156) which worked at the time, however in the more recent versions of Chrome it causes the content in RTL mode to shift whenever a child has a transform that is being animated (in our case it's usually ripples). These changes revert the backface-visibility in order to avoid the jumping, until we can find a better solution.

Relates to #10023.

A couple of examples of the issue:
1
2

A while ago we added a `backface-visibility` to some components that have scrollable content in order to avoid repaints while scrolling (see angular#7889, angular#7721, angular#7719, angular#6890, angular#2156) which worked at the time, however in the more recent versions of Chrome it causes the content in RTL mode to shift whenever a child has a transform that is being animated (in our case it's usually ripples). These changes revert the `backface-visibility` in order to avoid the jumping, until we can find a better solution.

Relates to angular#10023.
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 19, 2018
@@ -17,9 +17,6 @@ $mat-menu-icon-margin: 16px !default;
@mixin mat-menu-base($default-elevation) {
@include mat-overridable-elevation($default-elevation);

// Prevents the content from repainting on scroll.
Copy link
Member Author

@crisbeto crisbeto Feb 19, 2018

Choose a reason for hiding this comment

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

I'm also open to keeping these and scoping them to something like &:not([dir='rtl']), but it could end up causing inconsistencies.

@mmalerba
Copy link
Contributor

Have we filed a bug against chrome for this? It seems like a bug

@jelbourn
Copy link
Member

jelbourn commented Feb 20, 2018

How big is the difference with and without the backface-visibility?

@crisbeto
Copy link
Member Author

@jelbourn In terms of scrolling performance it promotes the element to a new GPU layer so it gets repainted less, however I haven't felt a noticeable difference when playing around with it.

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

We should also file an issue w/ Chrome, but if it doesn't make that much of a difference then I'm fine with this

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Feb 20, 2018
@crisbeto crisbeto added the target: patch This PR is targeted for the next patch release label Feb 20, 2018
@jelbourn jelbourn merged commit 900716a into angular:master Feb 26, 2018
mmalerba pushed a commit to mmalerba/components that referenced this pull request Feb 26, 2018
)

A while ago we added a `backface-visibility` to some components that have scrollable content in order to avoid repaints while scrolling (see angular#7889, angular#7721, angular#7719, angular#6890, angular#2156) which worked at the time, however in the more recent versions of Chrome it causes the content in RTL mode to shift whenever a child has a transform that is being animated (in our case it's usually ripples). These changes revert the `backface-visibility` in order to avoid the jumping, until we can find a better solution.

Relates to angular#10023.
jelbourn pushed a commit to jelbourn/components that referenced this pull request Feb 27, 2018
)

A while ago we added a `backface-visibility` to some components that have scrollable content in order to avoid repaints while scrolling (see angular#7889, angular#7721, angular#7719, angular#6890, angular#2156) which worked at the time, however in the more recent versions of Chrome it causes the content in RTL mode to shift whenever a child has a transform that is being animated (in our case it's usually ripples). These changes revert the `backface-visibility` in order to avoid the jumping, until we can find a better solution.

Relates to angular#10023.
@eimanip
Copy link

eimanip commented Aug 16, 2018

The bug still exists (Angular Material v6.4.5, Chrome v68).
You can see the problem here: https://stackblitz.com/edit/angular-ipohzl
The layout of Sidenav, Accordation and Select components will jump on selecting an item in rtl direction, if the content is scrollable.

@crisbeto
Copy link
Member Author

@eimanip you can solve most of the jumping by setting the dir through HTML, rather than CSS (see https://stackblitz.com/edit/angular-ipohzl-surdup). Setting it through CSS isn't supported by Material anyway, which is why the components look weird. That being said, the sidenav content still jumps even if the dir is set correctly. I'll do separate PR for the sidenav.

crisbeto added a commit to crisbeto/material2 that referenced this pull request Aug 18, 2018
* Fixes the content of the sidenav jumping around in RTL if it has active animations.
* Fixes text inside the sidenav being blurry on IE and Edge.

Relates to angular#10026.
@eimanip
Copy link

eimanip commented Aug 19, 2018

@crisbeto Thanks for your response. Hope this issue will be fixed.

jelbourn pushed a commit that referenced this pull request Aug 22, 2018
* Fixes the content of the sidenav jumping around in RTL if it has active animations.
* Fixes text inside the sidenav being blurry on IE and Edge.

Relates to #10026.
jelbourn pushed a commit that referenced this pull request Aug 29, 2018
* Fixes the content of the sidenav jumping around in RTL if it has active animations.
* Fixes text inside the sidenav being blurry on IE and Edge.

Relates to #10026.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants