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

RTL support? #274

Closed
HarelM opened this issue May 2, 2017 · 13 comments · Fixed by #527
Closed

RTL support? #274

HarelM opened this issue May 2, 2017 · 13 comments · Fixed by #527
Labels
enhancement has pr A PR has been created to address this issue P5 Low-priority issue that needs consideration
Projects

Comments

@HarelM
Copy link

HarelM commented May 2, 2017

I'm not sure if I'm configuring this incorrect of that this library doesn't support RTL content.
The following is my layout html:

<div fxLayout="row" fxLayoutAlign="space-between none">
        <div fxFlex="20">
            <md-slide-toggle [(ngModel)]="advanced"></md-slide-toggle>
        </div>
        <div fxFlex="60">
            <span (click)="advanced = !advanced"><i class="fa icon-gear"></i> {{resources.toggleAdvancedSettings}}</span>
        </div>
        <div fxFlex>
            <div fxLayout="col" fxLayoutAlign="start end">
                <span class="cursor-pointer" (click)="closeSidebar()"><i class="fa fa-lg icon-close"></i></span>
            </div>
        </div>
    </div>

What I want to accomplish is close to the following:
image
Only the "X" in the image should be aligned to the end, which in RTL case is left instead of right.
I'm using angular 4 with [dir]="rtl" directive so I was hoping this is in sync with angular's RTL support...

@HarelM
Copy link
Author

HarelM commented May 2, 2017

Sorry, my mistake.
Here is the correct code:

<div fxLayout="row" fxLayoutAlign="none none">
        <div fxFlex="20">
            <md-slide-toggle [(ngModel)]="advanced"></md-slide-toggle>
        </div>
        <div fxFlex="60">
            <span (click)="advanced = !advanced" class="cursor-pointer"><i class="fa icon-gear"></i> {{resources.toggleAdvancedSettings}}</span>
        </div>
        <div fxFlex>
            <div fxLayout="col" fxFill>
                <span fxFill fxLayoutAlign="end start" class="cursor-pointer" (click)="closeSidebar()"><i class="fa fa-lg icon-close"></i></span>
            </div>
        </div>
    </div>

@HarelM HarelM closed this as completed May 2, 2017
@HarelM
Copy link
Author

HarelM commented May 2, 2017

Well, now I'm not sure again...
The offset seems to be in the wrong place:
image
The css of the above elements shows:

element.style {
    flex: 1 1 100%;
    box-sizing: border-box;
    -webkit-box-flex: 1;
    max-width: 75%;
    margin-left: 15%;
}

setting margin-right:15% solves this, but I'm not sure how to really solve this, does seems like a bug.
the following is the html:

<div fxFlexOffset="15" fxFlex="75">
            <span>{{resources.addBaseLayer}}</span>
        </div>

@HarelM HarelM reopened this May 2, 2017
@ThomasBurleson
Copy link
Contributor

@HarelM - We need a Plunkr to test and validate against. Thx.

@HarelM
Copy link
Author

HarelM commented May 25, 2017

remove the dir="'rtl'". when putting it back on you should get a mirror image of the ltr version.
https://plnkr.co/edit/Pdmk94iCqGjmquDPcfU3?p=preview
This is not the case.
I haven't fully checked all other scenarios and it does feel that it's not exactly mirroring as it should but I don't have another case to show currently.

@ThomasBurleson
Copy link
Contributor

ThomasBurleson commented May 26, 2017 via email

@Eddygn
Copy link

Eddygn commented Oct 28, 2017

Hi,

Any (non manual CSS) workaround/solution for this issue ?

https://plnkr.co/edit/HOuHtMbH6mznoQpgHl6a?p=preview

@HarelM
Copy link
Author

HarelM commented Oct 31, 2017

The following is another issue I now see with the RTL support:

FlexLayoutGap="10px" inside a [dir]="'rtl'"
margin-right="10px" is added

Expected behavior:
margin-left="10px" should be used in this case

Version: 2.0.0-beta.9

basically the following code, thought not fully tested:

<div  [dir]="'rtl">
  <div fxLayout="row" fxLayoutGap="10px">
    <div fxFlex="70">
      the gap is before this element instead of after
    </div>
    <div fxFlex>
       Should be a gap between this and the previous one
    </div>
  </div>
</div>

@axmad22
Copy link

axmad22 commented Nov 12, 2017

Did you find a solution to this, by the way this happens even with fxLayout="row-reverse", it places the gap on the wrong side

@HarelM
Copy link
Author

HarelM commented Nov 12, 2017

The workaround I found was to use css instead, I know this is a poor workaround but unfortunately I don't have time to fix the bug here...

@ThomasBurleson ThomasBurleson modified the milestones: Backlog, v5.0.0-rc.1 Nov 28, 2017
@ThomasBurleson ThomasBurleson added bug enhancement P5 Low-priority issue that needs consideration and removed bug labels Nov 29, 2017
@atscott
Copy link
Contributor

atscott commented Dec 6, 2017

Can I ask why this was set to P5? This seems very important in order to support RTL languages.

@Eddygn
Copy link

Eddygn commented Dec 6, 2017

Looks like the plunkers no longer work, so here the same with the stackblitz template.

https://stackblitz.com/edit/angular-flex-layout-seed-eb7gve?file=app/test.component.ts

As of now, unfortunately, in practice it can't be used in RTL, it's easier to just use pure CSS (or any other solution).

@atscott
Copy link
Contributor

atscott commented Dec 7, 2017

Thanks for the demo!
I would really like to help fix this problem, as it's more like a P1 for any team (including my own) that needs to support RTL.
At least a partial fix seems relatively straightforward. The material2 library has this injectable for looking at the direction on the document/body: https://github.com/angular/material2/blob/master/src/cdk/bidi/directionality.ts
If this library had a similar one and then simply set margin-left in layout-gap instead of margin-right and margin-right instead of margin-left in flex-offset, that would be really helpful.

Unfortunately, I can't get the tests to run so I can't really make much progress on this myself.

Edit/Update: I fixed the broken tests by installing rxjs 5.5.0 instead of 5.5.5 so I'll be working on this today.

atscott added a commit to atscott/flex-layout that referenced this issue Dec 7, 2017
atscott added a commit to atscott/flex-layout that referenced this issue Dec 7, 2017
atscott added a commit to atscott/flex-layout that referenced this issue Jan 2, 2018
Apply margins correctly in rtl for flex-gap and flex-offset. fix for angular#274
atscott added a commit to atscott/flex-layout that referenced this issue Jan 2, 2018
Apply margins correctly in rtl for flex-gap and flex-offset. fix for angular#274

Apply margins correclty in rtl for flex-gap and flex-offset. fix for angular#274
@CaerusKaru CaerusKaru added has pr A PR has been created to address this issue in-progress and removed needs: demo labels Jan 3, 2018
@CaerusKaru CaerusKaru added this to Pending in API Fixes via automation Jan 17, 2018
@CaerusKaru CaerusKaru moved this from Pending to In-Progress in API Fixes Jan 17, 2018
API Fixes automation moved this from In-Progress to Done Feb 13, 2018
@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 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement has pr A PR has been created to address this issue P5 Low-priority issue that needs consideration
Projects
API Fixes
  
Done
Development

Successfully merging a pull request may close this issue.

6 participants