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(rtl): add segment rtl support #11215

Merged
merged 2 commits into from
Apr 14, 2017
Merged

Conversation

AmitMY
Copy link
Contributor

@AmitMY AmitMY commented Apr 13, 2017

Short description of what this resolves:

add segment rtl support

Changes proposed in this pull request:

Fixes:
image

Ionic Version: 3.x

@brandyscarney
Copy link
Member

Only issue I see with this is the thicker border width for the last 2 segments when there are more than 2 segment buttons:

screen shot 2017-04-13 at 4 58 44 pm

I have to head out for a bit but I can look into fixing it later or tomorrow if you don't. :)

@AmitMY
Copy link
Contributor Author

AmitMY commented Apr 13, 2017

@brandyscarney I think that fixes it.
Normally, everything has a border right, and only first need border left. here first does not need the left because it is (in ltr perspective) last

@brandyscarney brandyscarney merged commit dd0b293 into ionic-team:master Apr 14, 2017
@brandyscarney
Copy link
Member

@AmitMY Looks great! Thanks! 😄

@AmitMY AmitMY mentioned this pull request Apr 19, 2017
11 tasks
@manucorporat
Copy link
Contributor

@brandyscarney @AmitMY I don't want to be touchy, but I think this is a over-complicated and difficult to maintain fix, why we just don't use:

[dir="rtl"] ion-segment {
  flex-direction: row-reverse;
}

?

@AmitMY
Copy link
Contributor Author

AmitMY commented Apr 22, 2017

@manucorporat I think I briefly considered it, then dismissed
The reason is that it reverses the order of the columns..

This is LTR:
image

This is RTL (before this fix):
image

And this is RTL with row-reverse:
image

Finally, this is RTL after this fix: (which is the desired behavior)
image

It might not look as desired in English, but in an RTL language like Hebrew you can feel the difference:
image

@manucorporat
Copy link
Contributor

@AmitMY ok, I think I get it. Sorry for the ignorance :)

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

Successfully merging this pull request may close these issues.

3 participants