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

Use direction instead of rtl flag #34507

Merged
merged 1 commit into from
Nov 23, 2018
Merged

Use direction instead of rtl flag #34507

merged 1 commit into from
Nov 23, 2018

Conversation

albertoalmagro
Copy link
Contributor

Summary

This patch improves readability by using direction instead of rtlflag, as CSS does.

I have also took the chance to:

  • Use .to_sym when getting direction's env_value as it could be a String.
  • Removed the call to FileUtils.rm as FileUtils.mv already overwrites the assets if it exists

Other Information

Continues: #34486
Gives credit to @paracycle

r? @rafaelfranca

@rails-bot rails-bot bot added the docs label Nov 22, 2018
@gmcgibbon
Copy link
Member

Does *name & name* work in the commit message? I know square braces are supported, but unsure about using asterisks.

@rafaelfranca
Copy link
Member

Not that I can see in the code. I think only [ works

@gmcgibbon
Copy link
Member

I just checked locally; it doesn't. @albertoalmagro can you fix the syntax on your commit message? Thanks!

Improve readability by using `direction` as CSS does.

More info: https://developer.mozilla.org/en-US/docs/Web/CSS/direction
Continues: #34486

[Alberto Almagro + Ufuk Kayserilioglu]
@albertoalmagro
Copy link
Contributor Author

Done @gmcgibbon @rafaelfranca ! I've written it now as [name + name].

Thanks for checking it 👏

@gmcgibbon gmcgibbon merged commit a190e8c into rails:master Nov 23, 2018
@rafaelfranca
Copy link
Member

This was not a doc change but since it is not related to production code it was fine to merge.

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

Successfully merging this pull request may close these issues.

None yet

3 participants