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 - scss helpers issue #74

Closed
redidizzy opened this issue Aug 17, 2020 · 2 comments
Closed

RTL - scss helpers issue #74

redidizzy opened this issue Aug 17, 2020 · 2 comments

Comments

@redidizzy
Copy link
Contributor

Well, i'm trying to fix all rtl related issues and i came accross a positionning one.. to make it simple here's a portion of code found in resources/sass/spec/utils/layout/helpers/positions.scss

@for $i from 0 through 4 {
  .t-#{$i} { top: #{$i}px; }
  .r-#{$i} { right: #{$i}px; }
  .b-#{$i} { bottom: #{$i}px; }
  .l-#{$i} { left: #{$i}px; }

  @if ($responsive == true) {
    @include generateResponsive() {
      .t-#{$i}\@#{$breakpointAlias} { top: #{$i}px; }
      .r-#{$i}\@#{$breakpointAlias} { right: #{$i}px; }
      .b-#{$i}\@#{$breakpointAlias} { bottom: #{$i}px; }
      .l-#{$i}\@#{$breakpointAlias} { left: #{$i}px; }
    }
  }
}

the rights and lefts should logically be inversed in rtl.. therefore, i created a positions.scss file in a newly created /resources/sass/rtl subfolder and imported it in the rtl.scss file, here's(a portion of) its content :

@for $i from 0 through 4 {
  .t-#{$i} { top: #{$i}px; }
  .r-#{$i} { left: #{$i}px; }
  .b-#{$i} { bottom: #{$i}px; }
  .l-#{$i} { right: #{$i}px; }

  @if ($responsive == true) {
    @include generateResponsive() {
      .t-#{$i}\@#{$breakpointAlias} { top: #{$i}px; }
      .r-#{$i}\@#{$breakpointAlias} { left: #{$i}px; }
      .b-#{$i}\@#{$breakpointAlias} { bottom: #{$i}px; }
      .l-#{$i}\@#{$breakpointAlias} { right: #{$i}px; }
    }
  }
}

this way, a <div class="r-1"></div> would have a left:1px; css property...

Now here's the issue, the way we import rtl makes it so that we import app.scss as well as rtl.scss
app.scss imports the first positions.scss file(located in resources/sass/spec/utils/layout/helpers/positions.scss)
then rtl.scss imports the newly created positions.scss file(located in /resources/sass/rtl)

Therefore, a <div class="r-1"></div> would have a left:1px(generated by the newly created positions.scss) AND a right:1px; property (generated by the old positions.scss)..
This would lead to many unwanted behaviours in rtl... I hope i made myself as clear as possible

@redidizzy
Copy link
Contributor Author

My proposition is that instead of telling the user to uncomment the rtl line in the head tag.. we should duplicate the files imported in app.scss, change them accordingly for RTL, import the duplicated and changed files to rtl.scss then ask the user to choose between importing app.css or rtl.css. This way we make sure to not have any conflict.

We can even add a configuration variable for rtl and import the rtl.css file only when it's set to true ! It feels(to me at least) more like how it should be done

@kossa
Copy link
Owner

kossa commented Aug 27, 2020

Go ahead

@kossa kossa closed this as completed Jan 22, 2021
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

No branches or pull requests

2 participants