LTR/RTL fixes #11905

wackerow · 2024-01-11T00:48:12Z

Description

Use LTR direction for content portion of pages without translated content available
- (Not implemented on all pages yet. Started with applying to layouts, and tutorials landing page)
Force child components that use "common" namespace (always available for a locale) to use native text direction, overriding any upstream LTR declarations if page content is untranslated
Fix search modal direction-responsive css styling
Fix direction-responsive absolute positioning for some components

Preview URL

https://deploy-preview-11905--ethereumorg.netlify.app/

Screenshots:

https://deploy-preview-11905--ethereumorg.netlify.app/ar/developers/docs
ex. RTL locale, but content not translated:

https://deploy-preview-11905--ethereumorg.netlify.app/fa/developers/docs
ex. RTL locale where content is translated (unchanged, but for comparison))

RTL search modal:

"common" i18n namespace will always be available. This forces using the locale text-direction for these components, to override any cascading LTR overrides from pages with English-only content

(patches illegible colors)

insetInlineStart/End Chakra-UI style props not being recognized; refactored to use `style` prop

netlify · 2024-01-11T00:48:17Z

✅ Deploy Preview for ethereumorg ready!

Name	Link
🔨 Latest commit	`22ee0d9`
🔍 Latest deploy log	https://app.netlify.com/sites/ethereumorg/deploys/65aaa5e60c542b000813e131
😎 Deploy Preview	https://deploy-preview-11905--ethereumorg.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

wackerow · 2024-01-11T15:46:22Z

(Initial Netlify build failed from unrelated reasons which caused the check to fail... re-ran and it passed. Preview link from passing build is above, but the check never reset)

wackerow · 2024-01-12T02:29:43Z

src/components/Hero/HubHero/index.tsx

-        insetStart={{ xl: "8" }}
+        style={{ insetInlineStart }}


This change was made throughout as the style props insetStart/End or insetInlineStart/End have not been working as expected with Chakra components. Using style fixes this, but then we lose access to the Chakra tokens. As an alternative to using css var(--eth-type-token) syntax, the useToken hook was used.

wackerow · 2024-01-12T02:35:02Z

src/components/Search/utils.ts

@@ -149,7 +151,7 @@ export const getSearchModalStyles = (): SystemStyleObject => ({
  },

  ".DocSearch-Hit-Select-Icon:focus, .DocSearch-Hit-Select-Icon:hover": {
-    color: "switchBackground",
+    color: "switchBackground", // TODO: Remove? Causing low contrast in dark mode


@nloureiro When hovering directly over the arrow we get this low-contrast fill on dark mode:

It's not as bad in light mode but is there a color token you would recommend that looks good in both?

Or maybe we just consider removing this hover effect since it only triggers when hovering over the arrow itself, but the action doesn't change compared to the rest of the box. This would be my preference.

wackerow · 2024-01-12T02:36:30Z

src/components/Search/utils.ts

These file changes affect the search modal

wackerow · 2024-01-12T02:37:49Z

src/components/Search/utils.ts

+      p: 0,
+      ps: 2,


p: 0 overrides existing padding-left from @docsearch/css, then reassigns with direction-responsive ps

src/components/Search/utils.ts

wackerow · 2024-01-12T02:41:44Z

src/components/TranslationBanner.tsx

      position="fixed"
-      zIndex="99"
+      zIndex="banner"


Fixes bug where menu contents (ie. UseCases pages) were overlaying this banner.

src/pages/_app.tsx

TylerAPfledderer

My thoughts regarding RTL support here 😄

TylerAPfledderer · 2024-01-16T02:12:38Z

src/components/Breadcrumbs/index.tsx

@@ -65,7 +70,7 @@ const Breadcrumbs = ({
    .slice(startDepth)

  return (
-    <Breadcrumb {...props}>
+    <Breadcrumb {...props} dir={dir}>


Most instances of the dir prop should now be removed as this is redundant and is taken care of under the hood in the Chakra component structure/theme or by the use of an inset* prop.

An exception is where flexbox is used for content positioning, where CSS is not smart enough to handle RTL automatically with flex props. (I will point out the exceptions)

See other comment about specifically why I'm overriding these at the component level... same applied to this one.

src/components/CodeModal.tsx

TylerAPfledderer · 2024-01-16T02:40:42Z

src/components/FeedbackCard.tsx

@@ -67,6 +71,7 @@ const FeedbackCard = ({ prompt, isArticle, ...props }: FeedbackCardProps) => {
      mt="8"
      w="full"
      {...props}
+      dir={dir}


This is an exception where the dir prop should be kept, as flexbox is controlling the content positioning here.

Honestly I'm not having any problems with flex boxes here...

I'll double check all of your comments, but the reason for these was not just to use the dir for the chosen locale, but to specifically do so at the component level for components that:

Exclusively use the "common" namespace for translations (which every language has before even being listed

Are (or potentially) nested inside page content which may end up NOT being translated.

In this case (ie. /ar/developers/docs/blocks) html could be set to dir="rtl", the main content div could be set with dir="ltr" (overriden because contentNotTranslated=true), but then some components (ie. FeedbackCard) are fully translated, which should again run rtl.

The change here addressed this by independently setting the dir based on locale, without relying on html[dir] since the content it's nested in may flip it back.

Got it. Agreed, I think we need this for these "global" components.

src/components/Trilemma/index.tsx

src/layouts/Docs.tsx

src/lib/types.ts

fixes direction of pointer emoji for RTL pages, taking into account if contentNotTranslated. updates component with latest TS conventions; removes passing relativePath prop, instead loading locally using useRouter. rm redundant isNext, using only isPrev bool to simplify and avoid conflicts

src/hooks/useLocaleDirection.ts

use useTranslation hook over Translation component

pettinarip

@wackerow amazing job!!!! thanks for the work done here. Looks really nice the RTL version!

==========
Few minor things that don't block this PR.

Small detail in the slider arrows, we shouldn't use rtl props here.
https://deploy-preview-11905--ethereumorg.netlify.app/fa/what-is-ethereum

Not aligned

The results from the search are not translated, right? if so, I think we shouldn't change the direction

wackerow · 2024-01-20T18:52:16Z

@pettinarip

Small detail in the slider arrows, we shouldn't use rtl props here.

We should, but we need to pass { direction: "rtl" | "ltr" } to the useEmblaCarousel({ direction }), and flip the icons with flipForRtl (scaleX(-1))

Screen.Recording.2024-01-20.at.10.49.29.mov

Will put a patch up for this in a separate PR.

The results from the search are not translated, right? if so, I think we shouldn't change the direction

Yeah, they should be translated (if available):

...but, these languages aren't translated 100%, so we're still going to end up with English results for that content (ie. Algolia parses /ar/whitepaper into the ar lang facet... if it has English content, this is included in the Arabic search results)

Can discuss how we'd like to approach this.

The Trophy icon I'll look into; also separate to this PR. Thanks for spotting!

pettinarip · 2024-01-23T15:50:09Z

Nice, thanks @wackerow

wackerow added 5 commits January 10, 2024 16:34

fix: layout dir LTR for untranslated content

2be7723

fix: force locale dir for "common" ns components

b34d6ba

"common" i18n namespace will always be available. This forces using the locale text-direction for these components, to override any cascading LTR overrides from pages with English-only content

fix: search modal direction-responsive css styling

0612956

(patches illegible colors)

fix: direction-responsive absolute positioning

42df63d

insetInlineStart/End Chakra-UI style props not being recognized; refactored to use `style` prop

fix: use dir LTR for untranslated tutorials page

6a816cb

github-actions bot added content 🖋️ This involves copy additions or edits review needed 👀 tooling 🔧 Changes related to tooling of the project labels Jan 11, 2024

Merge branch 'dev' into nx-ltr-en-content

74524fa

wackerow commented Jan 12, 2024

View reviewed changes

wackerow marked this pull request as ready for review January 12, 2024 02:43

wackerow requested review from corwintines, pettinarip, minimalsm and nhsz as code owners January 12, 2024 02:43

wackerow added 5 commits January 15, 2024 14:57

fix: pass direction to Chakra theme provider

82e4094

refactor: use inset* style props

2557a20

Merge branch 'dev' into nx-ltr-en-content

9e5cd80

refactor: use inset* style props

832fb20

fix: use xl breakpoint

e4d96a3

wackerow commented Jan 15, 2024

View reviewed changes

src/pages/_app.tsx Outdated Show resolved Hide resolved

TylerAPfledderer suggested changes Jan 16, 2024

View reviewed changes

wackerow mentioned this pull request Jan 17, 2024

style: align toc item indicator with start border #11957

Draft

wackerow commented Jan 18, 2024

View reviewed changes

src/hooks/useLocaleDirection.ts Show resolved Hide resolved

wackerow added 2 commits January 18, 2024 15:23

chore: remove unnecessary props

40d9d1a

fix: i18n namespace

22ee0d9

use useTranslation hook over Translation component

pettinarip approved these changes Jan 19, 2024

View reviewed changes

wackerow mentioned this pull request Jan 20, 2024

fix: rtl support for embla carousel (Slider component) #11995

Merged

pettinarip merged commit b4106cd into dev Jan 23, 2024
9 of 10 checks passed

pettinarip deleted the nx-ltr-en-content branch January 23, 2024 15:50

This was referenced Jan 31, 2024

Release candidate v8.0.2 #12074

Merged

Deploy v8.0.2 #12079

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LTR/RTL fixes #11905

LTR/RTL fixes #11905

wackerow commented Jan 11, 2024 •

edited

netlify bot commented Jan 11, 2024 •

edited

wackerow commented Jan 11, 2024

wackerow Jan 12, 2024

wackerow Jan 12, 2024

wackerow Jan 12, 2024

wackerow Jan 12, 2024

wackerow Jan 12, 2024

pettinarip Jan 19, 2024

TylerAPfledderer left a comment

TylerAPfledderer Jan 16, 2024

wackerow Jan 18, 2024

TylerAPfledderer Jan 16, 2024

wackerow Jan 18, 2024

pettinarip Jan 19, 2024

pettinarip left a comment

wackerow commented Jan 20, 2024

pettinarip commented Jan 23, 2024

LTR/RTL fixes #11905

LTR/RTL fixes #11905

Conversation

wackerow commented Jan 11, 2024 • edited

Description

Preview URL

Screenshots:

netlify bot commented Jan 11, 2024 • edited

✅ Deploy Preview for ethereumorg ready!

wackerow commented Jan 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TylerAPfledderer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pettinarip left a comment

Choose a reason for hiding this comment

wackerow commented Jan 20, 2024

pettinarip commented Jan 23, 2024

wackerow commented Jan 11, 2024 •

edited

netlify bot commented Jan 11, 2024 •

edited