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

MWPW-125482 Add dir property to rtl locales #10

Closed
wants to merge 1 commit into from

Conversation

aishwaryamathuria
Copy link

Context

https://github.com/orgs/adobecom/discussions/746

  1. If a page with right-to-left content is previewed on Firefox browser then it gets rendered with left-to-right text flow.
  2. When EN content is used in an RTL locale page, there are instances when the locale of the page is en and content in that locale page needs to be aligned from left to right. Solution was to provide an option override the default rtl alignment.

Why this PR?

adobecom/milo#777
The above mentioned issue and requirement are fixed in milo in the shared PR.

  1. For the alignment issue a new property is added in the content.locales list with each locale named 'dir'. This property is used in place of the current Intl object which was unable to set the dir property in Firefox browser. To make sure the fix works for the project the same property needs to be set in the content.locales for rtl locales so that the issue can be resolved in Firefox browser.

How to use override?

Add the 'content-direction' meta tag with value 'rtl' or 'ltr' in the metadata block of the page or via metadata.xlsx to override the default text alignment.

Resolves: MWPW-125482

Test URLs:

@aem-code-sync
Copy link

aem-code-sync bot commented May 26, 2023

Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-run PSI Checks

@aishwaryamathuria
Copy link
Author

@meganthecoder @JasonHowellSlavin @Brandon32
I do not have the access on the repo to add the approvers. Could you review the PR?
It has similar changes as adobecom/bacom#118
In case the locale is not supported on bacom-blog and the change is not required could you confirm the same and close the PR?

@Dli3 Dli3 self-assigned this Jun 7, 2023
@meganthecoder
Copy link
Contributor

@JasonHowellSlavin @Brandon32 I don't believe the blog uses these locales. Do we want to close this PR or merge it in order to be aligned with milo and bacom?

@JasonHowellSlavin
Copy link
Contributor

@JasonHowellSlavin @Brandon32 I don't believe the blog uses these locales. Do we want to close this PR or merge it in order to be aligned with milo and bacom?

I think I may close this since we do not use those locales on bacom-blog, and our locales seem to be out of date anyway. Should we open a new PR to match bacom?

@meganthecoder
Copy link
Contributor

@JasonHowellSlavin @Brandon32 I don't believe the blog uses these locales. Do we want to close this PR or merge it in order to be aligned with milo and bacom?

I think I may close this since we do not use those locales on bacom-blog, and our locales seem to be out of date anyway. Should we open a new PR to match bacom?

Good idea. I created an issue for this #58

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.

4 participants