RTL (right-to-left): change to logical CSS

This issue remains relevant because Gin should adopt CSS logical properties. However, the code in the current issue fork is quite outdated compared to the latest Gin version. It would make more sense to open a new issue fork against the current Gin codebase to implement CSS logical property support.

saschaeggi → closed merge request !140

This needs a new review 👀

What a switch of logic. nearly half of the code will go!!

My first quick review of the admin pages in RTL is good.
HTML tags and elements look spaced, positioned, and bordered correctly.

• Having the 2nd full testing round
• Automated functional testing round

I had the 2nd full testing round with the following extra Gin modules.
and webform, dashboards, and responsive preview

        "cweagans/composer-patches": "~1 || ~2",
        "drupal/gin": "3.x-dev",
        "drupal/gin_login": "~2",
        "drupal/gin_toolbar": "~1",
        "drupal/gin_moderation_sidebar": "~1",
        "drupal/gin_everywhere": "~1",
        "drupal/gin_type_tray": "~1",
        "drupal/dashboards": "~2",
        "drupal/webform": "~6",
        "drupal/responsive_preview": "~2"

and

        "patches": {
          "drupal/gin": {
            "Issue #3279620 by saschaeggi: RTL (right-to-left): change to logical CSS":
            "https://git.drupalcode.org/project/gin/-/merge_requests/427.diff",
            "Issue #3398040: Fix issue with changing between responsive views and Gin Toolbar over menus":
            "https://www.drupal.org/files/issues/2023-10-31/3398040-8.patch"
          },
          "drupal/gin_everywhere": {
            "Issue #3436449: The status tab is showing out in the Are you sure you want to delete the content item page":
            "https://www.drupal.org/files/issues/2024-05-15/gin_everywhere--2024-05-15--3436449-5.patch"
          },
          "drupal/gin_type_tray": {
            "Issue #3448075: Fix the padding for filter form wrapper div":
            "https://www.drupal.org/files/issues/2024-05-19/gin_type_tray--2024-05-19--3448075-2.patch",
            "Issue #3450665: Fix: Twig Error Loader Error when attempting to add new content.":
            "https://www.drupal.org/files/issues/2024-06-02/gin_type_tray--2024-06-02--3450665-7.patch"
          }
        },

Faced issues with:

- The drupal-off-canvas opens at the right side for Layout Builder at the back-end ( example dashboard )

- Admin Gin Toolbar Toggle icon is flipped.
- The drupal-off-canvas opens at the right side for Moderation sidebar toolbar with ( Gin Moderation Sidebar )
→

- Back to content editing in Content Preview of a content in RTL
→

- Webform admin page ( Drupal 10.3.0 ) in both LTR and RTL.
→

Webform admin page ( Drupal 10.3.0 ) in both LTR and RTL.

🐛 sticky table incompatible with Core 10.3+ (e.g. Permissions) RTBC should fix this 👀

Noted;
Having a 3rd full testing round, with more modules and more integrated modules with Gin.
Watching 👀 the full Automated functional testing round for our product
I may spot more issues.

Spotted an issue with views exposed form items with VBO and VBO Editor
When adding more exposed filters or better exposed filters like

LTR - Admin Content page with more better exposed filters

RTL - Admin Content page with more better exposed filters

Maybe Gin has to reset all padding, margin-top or others from Claro
Maybe any custom styling or fixing for custom Exposed Filters ( or better exposed filters )

themes/contrib/gin/dist/css/base/gin.css

.views-exposed-form .form-item--no-label, .views-exposed-form__item.views-exposed-form__item.views-exposed-form__item--actions {
  margin-block: var(--gin-spacing-s) 0;
  align-self: flex-end;
}

core/themes/claro/css/components/views-exposed-form.css

.views-exposed-form .form-item--no-label, .views-exposed-form__item.views-exposed-form__item.views-exposed-form__item--actions {
  margin-top: calc(var(--line-height-form-label) + var(--space-s) + var(--space-xs));
}
.views-exposed-form__item.views-exposed-form__item {
  max-width: 100%;
 - margin-block: var(--space-s) 0;-
  margin-inline: 0 var(--space-xs);
}

Back to Gin without custom styling. or having custom Claro style from Claro or custom modules

I have finished the 3rd full testing round.

No more issues to report at this time. Only the ones I reported.
They are only for 3rd party integrations.
Only our custom fixes or custom styling with integrated modules ( to manage that on our own time )
Drupal core + new changes have no break completely ( I did not notice any )

Thank you for bringing me into this.
This logic will be implemented in other themes or any custom styling.
By the way ( using a copy of your webpack.config with some customs in many many drupal projects, and themes.

For the future work.

Yesterday late-night. I had a deep scan reading for the https://www.w3.org/TR/css-logical-1
Most themes should change to this logic (planning to do that with Bootstrap 5, maybe not wait for Bootstrap 6)
For sure it will need time.

1. I feel it is safe to merge and release in 8.x-3.x and 4.0.x , although having this in 4.0.x will give notice and time to all Gin integrators to do the fixes to work with Gin ~4.0
2. Contributors can manage small fixes for other integrated modules.
3. Developers/themers can manage small fixes for custom-coded fixes in projects.
4. Important to follow up with the Drupal Core team to adopt logical properties ( I think a session in DrupalCon, will start a movement for this )
5. It could be 100% done in Drupal 11 or 12 + Gin 4

I have tested the RTL. i found few issues other than that translation working fine other than these issues. i have attached issue screenshot.
1. full stop should come at last of the sentence.
2. after selecting arabic from language when you goto user page and edit user site language will change automatically to english.

@saurav-drupal-dev as both issues are not coming from Gin I think this should be fine for now

@rajab natshah I think as there is now a lot of momentum for Drupal Starshot we can proceed with this even in 3.x

3.0 will be 4.0 at the same time. As Gin is not yet marked as stable and as you reported there are no major issues I think we should be good to go after a rebase. This will simplify the CSS codebase quite a bit.

@rajab let me know if you have any objections, I've just rebased the MR so this would need a final look 👀

Thank you, Sascha, for following up on this issue :)

I for sure agree to move with Logical Properties 8.x-3.x and 4.0.x I had full testing rounds for that.

We will follow with your method in Logical Properties in all other back-end and front-end styling.

Hi guys,
Really happy to see this work happening after 3 years !

I see some more improvements could be done, but maybe it's intentional to leave it like this.

For example I can see in generated css

1. position right or left

[dir="ltr"] .gin-sticky-form-actions .gin-more-actions__menu.form-actions {
  right: 0;
}

[dir="rtl"] .gin-sticky-form-actions .gin-more-actions__menu.form-actions {
  left: 0;
}

That could be replaced by

.gin-sticky-form-actions .gin-more-actions__menu.form-actions {
  inset-inline-end: 0;
}

2. text-align

text-align: left could be replaced by text-align: start

3. margin or padding with 4 or 3 values

[dir="ltr"] .form--inline .form-item,
[dir="ltr"] .form--inline .form-actions,
[dir="ltr"] [data-drupal-selector*=-bulk-form] .form-item,
[dir="ltr"] [data-drupal-selector*=-bulk-form] .form-actions,
[dir="ltr"] .layout-region-node-footer__content .form-item,
[dir="ltr"] .layout-region-node-footer__content .form-actions {
  margin: var(--gin-spacing-xs) var(--gin-spacing-xs) var(--gin-spacing-xs) 0;
}

[dir="rtl"] .form--inline .form-item,
[dir="rtl"] .form--inline .form-actions,
[dir="rtl"] [data-drupal-selector*=-bulk-form] .form-item,
[dir="rtl"] [data-drupal-selector*=-bulk-form] .form-actions,
[dir="rtl"] .layout-region-node-footer__content .form-item,
[dir="rtl"] .layout-region-node-footer__content .form-actions {
  margin: var(--gin-spacing-xs) 0 var(--gin-spacing-xs) var(--gin-spacing-xs);
}

Could be replaced by

.form--inline .form-item,
.form--inline .form-actions,
[data-drupal-selector*=-bulk-form] .form-item,
[data-drupal-selector*=-bulk-form] .form-actions,
.layout-region-node-footer__content .form-item,
.layout-region-node-footer__content .form-actions {
  margin-block: var(--gin-spacing-xs);
  margin-inline:  0 var(--gin-spacing-xs);
}

@heyyo that was potentially not covered when I rebased it as they were newer changes. I pushed a new change to add this to the coverage.

Thanks for investing more effort into it.
This logical postCSS processor is also a great addition to guarantee only logical css will be used.

I found 2 RTL issues related on mobile, but not related to logical css,
- one related to shorcut message displayed when pressing the star, because this text is not really hidden(opacity:0), it takes too much space if the translation is too long, so it breaks the page and add unecessary extra horizontal scroll
- The second is related to gin tooltip for example the one on hovering menu items in navigation bar, in RTL they are truncated, because they are not going to the correct direction (position set in javascript)
This gin-tooltip position could affect completly the edition of entity, I had one example where position was to left -1200px on mobile, so at first load page is completly blank so we need to scroll horizontally !

I suppose no javascript is needed to display tooltips now days.

Here a great resource which shows how to display tooltip even for LTR/RTL in pure css
https://web.dev/articles/building/a-tooltip-component

Thanks, Lionel, for having further testing and checking.

WOW, so nice.

This logical postCSS processor is also a great addition to guarantee only logical css will be used.

Thanks for the hint

Maybe we have to convert all other Gin integrations, or some front-end themes with that.
Is the following what you mean?

    "postcss": "^8.4.47",
    "postcss-rtl-logical-properties": "^1.0.7",

const postcssRtlLogicalProperties = require('postcss-rtl-logical-properties');

              postcssOptions: {
                plugins: [
                  autoprefixer(),
                  postcssRtlLogicalProperties(),

Would it be a good session or a video shared about this?

The second is related to toolbar tooltip for example the one on hovering menu items in navigation bar, in RTL they are truncated, because they are not going to the correct direction (position set in javascript)

This was fixed yesterday with the latest push

postcss-rtl-logical-properties

Just makes sure that when CSS gets compiled that we don't introduce new non-logical CSS code (as the pipeline will fail and compiled CSS will always include logical CSS)

I suppose no javascript is needed to display tooltips now days.

Yes, this might be something to look at (also in Core) as we're currently using the integration of Core's new navigation tooltip library to display them (to use a unified approach).

one related to shorcut message displayed when pressing the star, because this text is not really hidden(opacity:0), it takes too much space if the translation is too long

This is something we can look into fixing here or in a follow-up but I guess this is not currently broken because of moving to logical CSS, right?

yes this bug with text with long translation, is not caused by logical css. I can see it in both version.
I shared with you guys, because now I'm working on a website which should work on mobile even for the administrative part, in english and in hebrew.

By reading this resource more carefully https://web.dev/articles/building/a-tooltip-component

I think we can resolve even more css rules, to avoid some "dir=rtl" "dir=ltr"

logical css doesn't support translateX, background position, box-shadow...

But there is a chance we can simulate logical css for those too, by using css variable and css calc

tool-tip {
  --isRTL: -1;
}

tool-tip:dir(rtl) {
  --isRTL: 1;
}
...
--_x: calc(var(--isRTL) * -3px * -1);

@heyyo should we move forward with what we have in this MR to unblock this? I think we can fix the Shortcut one in a follow-up as it wasn't introduced here. WDYT?

Hi @saschaeggi
I completely agree with you, the actual MR is a huge step to improve RTL support and to make gin css styles really lighter.

Thanks for testing & providing feedback on this @heyyo & rajab natshah 👏

Automatically closed - issue fixed for 2 weeks with no activity.