search

jossef / material-design-icons-iconfont

Material Design icons + Development Experience
https://jossef.github.io/material-design-icons-iconfont
Apache License 2.0
455 stars 58 forks source link

web: icons do not follow page ltr/rtl direction #49

Closed clshortfuse closed 2 years ago

clshortfuse commented 4 years ago

Thanks so much for maintaining this repo!

This is a replicated issue from https://github.com/google/material-design-icons/issues/750

In summary direction:ltr tells the browser where to position an icon, not how the icon is drawn. It's forcing left-to-right (LTR) even on RTL (right-to-left) layouts. The line should be removed.

Let me start of by saying, I'm not talking about glyph/icon mirroring.

When a page is using [dir=rtl] items should flow from right to left. Material Design Icons is forcing the orientation to be left to right, even on pages that should be right to left. This forces you to div wrap the icon for the layout to flow properly.

The line in question, specifically, is this one:

https://github.com/jossef/material-design-icons-iconfont/blob/25db3c0d5825bd09237fad6e4af750b80b7c4fd6/src/material-design-icons.scss#L33

Here's the codepen explaining the issue:

https://codepen.io/shortfuse/pen/RMgrOw

You can see the warning icon isn't properly respecting the page's RTL implementation.

I'm not sure why the direction: ltr since no browser actually mirrors the icons. This is even stated in the guide:

By default on the web, icons are not mirrored when the layout direction is mirrored. You need to specifically mirror the appropriate icons when needed.

Perhaps the usage of direction: ltr was misunderstood when the rule was added. If it is fully intentional, there should be a better explanation as to why it has to be used and how to workaround the layout direction issue.

jossef commented 4 years ago

@clshortfuse nice catch would you like to make a PR?

jossef commented 2 years ago

I want to remove the direction: ltr.

I double-checked with the original font's CSS This legacy is coming from there.

keeping this issue open for now. image

clshortfuse commented 2 years ago

You can either remove it, or use direction: inherit; which allows RTL pages to work properly. I use inherit instead of removing to ensure the rule is changed.

The codepen still propertly shows the issue. I'll make the PR for you.