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

Icon: Add right-to-left locales flip style support and update doc #631

Merged
merged 4 commits into from Jan 22, 2020

Conversation

selena-zeng
Copy link
Contributor

@selena-zeng selena-zeng commented Jan 17, 2020

TODO

  • Documentation
  • Tests
    Experimental evidence (required for Masonry changes)
  • Accessibility checkup

image

@selena-zeng selena-zeng requested a review from a team as a code owner January 17, 2020 21:41
@christianvuerings christianvuerings changed the title Icon Add right-to-left locales flip style support and update doc Icon: Add right-to-left locales flip style support and update doc Jan 21, 2020
flip on RTL locales like Arabic). Consider whether you want to keep the
original layout on rtl if you are adding new icons.
</Text>
<Box display="flex" direction="row">
Copy link
Contributor

Choose a reason for hiding this comment

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

@selena-zeng Let's remove this Box since the rtl will be a feature across pages

@@ -34,6 +34,20 @@ type Props = {|

const IconNames = Object.keys(icons);

const noFlipOnRtlIconNames = [
'facebook',
Copy link
Contributor

Choose a reason for hiding this comment

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

@selena-zeng this is different from the approach we talked about - is this in agreement with PDS, that only these icons should not be flipped and all the other ones should?

html[dir="rtl"] .rtlSupport {
/* Flip the icon for right-to-left language locales */
transform: rotateY(180deg);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@selena-zeng could we add an extra breaking line at the end of this file?

image

@christianvuerings christianvuerings merged commit 73c95fd into pinterest:master Jan 22, 2020
@christianvuerings
Copy link
Contributor

@selena-zeng Thank you for working with PDS on this - merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants