-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix(rtl): support applying dir="rtl" on DOM elements other than the body #11579
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just have a few minor requests for changes and a question.
It would be good to add some documentation for this feature somewhere. Perhaps in a Localization Guide in docs/content/localization
using Markdown?
@marosoft can you please rebase this PR and address the review comments when you get a chance? |
c378cb7
to
d1b4126
Compare
@Splaktar I noticed that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggested some improvements to the JSDoc.
Thanks for the notes for the Migration guide. Would you be interested in adding those to the guide in another PR after this one gets merged?
Create a single function to check text direction based on dir attribute on DOM element or document. Fixes angular#11016 Fixes angular#9754
d1b4126
to
cc98e60
Compare
@Splaktar I made the suggested changes, thanks for those. |
No issue needed. Please just reference this PR. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would really like some basic unit tests to verify this functionality if possible, but I don't want to block getting this in for many weeks. Please let me know if you can add a couple basic tests soon, otherwise, can you please open an issue to add tests for this?
Also can you please open an issue to create a new Localization Guide in docs/content/localization
(using Markdown)?
Thank you for this important work!
Ideally, it would be nice to see a couple tests for each affected component (in their Affected components
Good catch on that RTL docs issue! |
Create a single function to check text direction based on dir attribute on DOM element or document.
Fixes #11016 Fixes #9754
PR Checklist
Please check that your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
A whole document is expected to be set as RTL even when a
dir
attribute is set properly on an individual element.Issue Number: #11016, #9754
What is the new behavior?
It is possible to use
dir
attribute on an element to override a document'sdir
value.Does this PR introduce a breaking change?
Other information