[UI] Add RTL support by Frenzie · Pull Request #2658 · FreshRSS/FreshRSS

Frenzie · 2019-11-12T11:32:31Z

Fixes #673.

Now obviously this only fixes one thing when there are at least a dozen similar issues, but this principle is my proposed solution to all of them.

Edit: to clarify, basically anything that says something like position: absolute or fixed is likely affected. So are border-left/right. The good news is that anything that isn't absolutely positioned should largely Just Work™.

This change is

Fixes FreshRSS#673.

Because holy @#$@# is that annoying. :-P

Frenzie · 2019-11-12T14:33:43Z

@mat-mo Does the previous/next article navigation in the bottom right look as expected? (I imagine it doesn't. ;-) )

Frenzie · 2019-11-12T15:29:18Z

Besides the act of adding the relevant attributes to the HTML, this PR should pretty much be in a decent enough shape to be merged for testing in the dev branch. It resolves most (hopefully all of course ;-) ) issues.

LTR

RTL

Alkarex · 2019-11-12T16:14:18Z

p/themes/Origine/origine.css

+	border-radius: 3px 0 0 3px;
+}
+
+html:not(.rtl) .stick .btn + .btn,


Can't we avoid the :not(.rtl) thanks to the .rtl below, overriding the properties?

Not quite elegantly. Options include border-left-width: 0; instead of none, so you can easily restore it to some px value (probably the best alternative) and possibly also some invalid value.

The alternative looks like this:

diff --git a/p/themes/Origine/origine.css b/p/themes/Origine/origine.css index 86210940..5f90fb1b 100644 --- a/p/themes/Origine/origine.css +++ b/p/themes/Origine/origine.css @@ -150,16 +150,16 @@ html.rtl .stick input:last-child { border-radius: 3px 0 0 3px; } -html:not(.rtl) .stick .btn + .btn, -html:not(.rtl) .stick .btn + input, -html:not(.rtl) .stick .btn + .dropdown > .btn, -html:not(.rtl) .stick input + .btn, -html:not(.rtl) .stick input + input, -html:not(.rtl) .stick input + .dropdown > .btn, -html:not(.rtl) .stick .dropdown + .btn, -html:not(.rtl) .stick .dropdown + input, -html:not(.rtl) .stick .dropdown + .dropdown > .btn { - border-left: none; +.stick .btn + .btn, +.stick .btn + input, +.stick .btn + .dropdown > .btn, +.stick input + .btn, +.stick input + input, +.stick input + .dropdown > .btn, +.stick .dropdown + .btn, +.stick .dropdown + input, +.stick .dropdown + .dropdown > .btn { + border-left-width: 0; } html.rtl .stick .btn + .btn, @@ -171,7 +171,8 @@ html.rtl .stick input + .dropdown > .btn, html.rtl .stick .dropdown + .btn, html.rtl .stick .dropdown + input, html.rtl .stick .dropdown + .dropdown > .btn { - border-right: none; + border-left-width: 1px; + border-right-width: 0; } .stick input + .btn {

It looks a bit lighter, maybe? No strong preference

I went with the current version because I figured it's slightly easier to parse (if not ltr then border-left:none, elseif ltr then border-right: none) but I'm fine with any approach. A similar option is to add html.ltr as well.

Alkarex · 2019-11-12T16:20:26Z

So we lack adding class="rtl", right? Should this be automatic based on the language selected?

Frenzie · 2019-11-12T16:26:38Z

Indeed. For testing I used this simple change, that should also be put in install.php.

diff --git a/app/layout/layout.phtml b/app/layout/layout.phtml
index 498cc447..d08261d7 100644
--- a/app/layout/layout.phtml
+++ b/app/layout/layout.phtml
@@ -1,6 +1,6 @@
 <?php FreshRSS::preLayout(); ?>
 <!DOCTYPE html>
-<html lang="<?= FreshRSS_Context::$user_conf->language ?>" xml:lang="<?= FreshRSS_Context::$user_conf->language ?>">
+<html lang="<?= FreshRSS_Context::$user_conf->language ?>" xml:lang="<?= FreshRSS_Context::$user_conf->language ?>" dir="rtl" class="rtl">
        <head>
                <meta charset="UTF-8" />
                <meta name="viewport" content="initial-scale=1.0" />

I was thinking it could perhaps be put in the app/i18n/<lang> folder as some kind of metadata, or alternatively it could simply be hardcoded in an array somewhere.

p/themes/Origine/origine.css

Alkarex · 2019-11-12T17:53:01Z

I suggest we add a line

'dir' => 'rtl',

in https://github.com/FreshRSS/FreshRSS/blob/dev/app/i18n/en/gen.php
were /en/ is to be replaced by RTL languages

Frenzie · 2019-11-12T19:52:12Z

@Alkarex Something like this? 0c06acb

app/layout/layout.phtml

Co-Authored-By: Alexandre Alapetite <[email protected]>

app/install.php

Alkarex · 2019-11-12T20:36:36Z

Yes @Frenzie , looks good 👍

Frenzie · 2019-11-12T22:58:28Z

I did this manually, but I just noticed https://github.com/cssjanus/cssjanus https://github.com/cssjanus/php-cssjanus

Which opens up an easy path to a completely different approach.

Frenzie · 2019-11-13T09:55:32Z

I'm thinking this CSSJanus approach may be better than this PR, mainly because it should require (almost) no effort to maintain, not to mention manually going through all the themes would add up even if it can be slowly spaced out.

Using the PHP port it could potentially even be integrated into the program itself.

The basic idea is a bit different though. Instead of localized overrides in the same CSS file, you'd generate a filename.ltr.css (or whatever you want to call it), and you'd load that one instead of the normal CSS file. There's an article about the principle illustrated with SASS here.

When minor touchups are required, the "current" approach (html.rtl etc) could still be used to perform specific overrides.

There are actually also newer CSS attributes that may mostly avoid the issue, but they're not supported by Edge yet.

https://drafts.csswg.org/css-writing-modes-4/#flow-relative
https://developer.mozilla.org/en-US/docs/Web/CSS/margin-block-start

marienfressinaud · 2019-12-07T12:00:55Z

Can we close this PR in favor of #2678?

Frenzie · 2019-12-07T15:18:24Z

I believe the other approach is much better in principle, yes.

[UI] Add RTL support

4a34b2d

Fixes FreshRSS#673.

Frenzie changed the base branch from master to dev November 12, 2019 11:38

Fix that menu even though this is just an illustration atm

0651331

Because holy @#$@# is that annoying. :-P

Frenzie added this to the 1.16.0 milestone Nov 12, 2019

Frenzie added the UI 🎨 User Interfaces label Nov 12, 2019

Frenzie mentioned this pull request Nov 12, 2019

Automatic mostly-right RTL with dir="auto" #2656

Merged

Frenzie added 6 commits November 12, 2019 14:34

fix install page

43f7a7a

flip order

f751565

your rss feeds message

2513678

fix up the buttons

b41862b

add empty lines

428d1f4

remove duplicate selector

f28459e

Frenzie added 5 commits November 12, 2019 15:54

fix feed menu

35eb48c

fix page number

19d37c9

fix stylelint

71df522

swap indicator border

f27bb9f

fix tree text-align

8070bae

Alkarex reviewed Nov 12, 2019

View reviewed changes

Alkarex approved these changes Nov 12, 2019

View reviewed changes

Alkarex marked this pull request as ready for review November 12, 2019 16:17

Frenzie commented Nov 12, 2019

View reviewed changes

p/themes/Origine/origine.css Show resolved Hide resolved

Frenzie added 2 commits November 12, 2019 17:50

fix btn-important

90a1a5a

fix order

77fc285

Add dir info to gen.php

0c06acb

forgot install.php!

4351e02

Alkarex reviewed Nov 12, 2019

View reviewed changes

app/layout/layout.phtml Outdated Show resolved Hide resolved

Update app/layout/layout.phtml

48345d8

Co-Authored-By: Alexandre Alapetite <[email protected]>

Frenzie commented Nov 12, 2019

View reviewed changes

app/install.php Outdated Show resolved Hide resolved

Update app/install.php

9ff6ab0

Alkarex mentioned this pull request Nov 12, 2019

RTL support #673

Closed

This was referenced Nov 16, 2019

Right to left support (Arabic, Hebrew, ...) koreader/koreader#5359

Closed

[UI] Add RTL support with CSSJanus #2678

Closed

Frenzie closed this Dec 7, 2019

Frenzie deleted the ui-rtl branch December 7, 2019 15:18

Uh oh!

Conversation

Frenzie commented Nov 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Frenzie commented Nov 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Frenzie commented Nov 12, 2019

LTR

RTL

Uh oh!

Alkarex Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

Frenzie Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

Alkarex Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

Frenzie Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

Alkarex Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

Frenzie Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

Alkarex commented Nov 12, 2019

Uh oh!

Frenzie commented Nov 12, 2019

Uh oh!

Uh oh!

Alkarex commented Nov 12, 2019

Uh oh!

Frenzie commented Nov 12, 2019

Uh oh!

Uh oh!

Uh oh!

Alkarex commented Nov 12, 2019

Uh oh!

Frenzie commented Nov 12, 2019

Uh oh!

Frenzie commented Nov 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marienfressinaud commented Dec 7, 2019

Uh oh!

Frenzie commented Dec 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Frenzie commented Nov 12, 2019 •

edited

Loading

Frenzie commented Nov 12, 2019 •

edited

Loading

Frenzie commented Nov 13, 2019 •

edited

Loading