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

bug(mdc-progress-bar): Does not flip with body set to dir="rtl" #22609

Closed
andrewseguin opened this issue May 1, 2021 · 8 comments · Fixed by #22705
Closed

bug(mdc-progress-bar): Does not flip with body set to dir="rtl" #22609

andrewseguin opened this issue May 1, 2021 · 8 comments · Fixed by #22705
Assignees
Labels
area: material/progress-bar P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@andrewseguin
Copy link
Contributor

The legacy progress bar has specific styles to flip its directionality based on a parent having [dir='rtl']:

  // Reverse the apparent directionality of progress vars for rtl.
  &[dir='rtl'],
  [dir='rtl'] & {
    transform: rotateY(180deg);
  }

  &[mode='query'] {
    transform: rotateZ(180deg);

    &[dir='rtl'],
    [dir='rtl'] & {
      transform: rotateZ(180deg) rotateY(180deg);
    }
  }

However, this is missing from the new MDC-based progress bar. Should this extra code be included? An internal test fails due to this case, but perhaps it should have been implemented otherwise through the Directionality service?

@andrewseguin andrewseguin added the needs triage This issue needs to be triaged by the team label May 1, 2021
@crisbeto
Copy link
Member

crisbeto commented May 1, 2021

Are you seeing the regression in our own dev app or someplace else? The dev app has a custom Directionality implementation which doesn't pick up the direction from the DOM.

@andrewseguin
Copy link
Contributor Author

yeah its an internal test - they just add dir="rtl" to the body and expect it to work

@crisbeto
Copy link
Member

crisbeto commented May 3, 2021

That should work, but I'll double-check. The issue I mentioned above is with our own dev app.

@crisbeto
Copy link
Member

crisbeto commented May 6, 2021

I gave it another shot, but it worked as expected. We might have to investigate that internal test.

@crisbeto crisbeto added needs investigation A member of the team needs to do further investigation to determine the root cause and removed needs triage This issue needs to be triaged by the team labels May 6, 2021
@andrewseguin
Copy link
Contributor Author

If you go into the demo app and just add dir="rtl" to the body, the legacy progress bar will flip but the legacy one will not. That should be supported right?

@crisbeto
Copy link
Member

The legacy one works, because the flip happens purely in CSS, whereas the MDC one has to execute some JS logic.

crisbeto added a commit to crisbeto/material2 that referenced this issue May 15, 2021
…ge directionality

On the first iteration of the MDC progress bar we had to use JS to change the direction in RTL, but a few months ago in angular#21650 we switched to a different API that no longer depends on it. The problem with the changes in angular#21650 is that they set the `dir` on the progress bar in order to flip its direction for the `query` mode.

These changes simplify the setup by relying only on CSS to determine the direction.

Fixes angular#22609.
@crisbeto
Copy link
Member

I looked into it again and it seems like we've moved away from the MDC API that required us to flip direction using JS. I've submitted #22705 which should resolve this issue.

@crisbeto crisbeto added area: material/progress-bar has pr P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed needs investigation A member of the team needs to do further investigation to determine the root cause labels May 15, 2021
@crisbeto crisbeto self-assigned this May 15, 2021
andrewseguin pushed a commit that referenced this issue May 18, 2021
…ge directionality (#22705)

On the first iteration of the MDC progress bar we had to use JS to change the direction in RTL, but a few months ago in #21650 we switched to a different API that no longer depends on it. The problem with the changes in #21650 is that they set the `dir` on the progress bar in order to flip its direction for the `query` mode.

These changes simplify the setup by relying only on CSS to determine the direction.

Fixes #22609.
@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 Jun 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: material/progress-bar P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
2 participants