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

Simple theme: implement RTL #750

Merged
merged 3 commits into from
Jan 16, 2022
Merged

Simple theme: implement RTL #750

merged 3 commits into from
Jan 16, 2022

Conversation

dalf
Copy link
Member

@dalf dalf commented Jan 13, 2022

What does this PR do?

This PR implements the RTL text direction in the simple theme.

Modern browsers do most of the work, but margin / padding / border and some other properties have to be manage manually.

The usual method seems to be:

#q {
  border-right: none;
}

#q [dir="rtl] {
  border-right: inherit;
  border-left: none;
}

IMO, this is error prone and require some copy / paste

There is the margin-inline-start property, but it seems to be supported only recently (except for FF), and it doesn't solve all cases (for example float: right).

So this PR uses some LESS mixins:

In LTR version:

.ltr-left(@offset) {
  left: @offset;
}

In this RTL version:

.ltr-left(@offset) {
  right: @offset;
}

and so on for the different CSS properties.

Why is this change important?

Add support for RTL languages in the Simple theme.

image

How to test this PR locally?

  • check everything in a LTR language
  • check everything in a RTL language

Author's checklist

Issue list:

  • URL in the result
  • Vim hotkeys help
  • LTR text in the result are displayed correctly.

For the last one: I dont' know if there is a way to display LTR text aligned on the left, and RTL text aligned on the right only using CSS.

Currently, the only way I see is to detect the text direction in Python, and then add the dir HTML attribute.

From that, it is possible to add some CSS to display the text either aligned on the right, either aligned on the left.

ping @GreenLunar

Related issues

Close #747

@mrpaulblack
Copy link
Member

@dalf I added some small fixes on top of your branch; feel free to change/throw away/whatever 👍

The things that still need attention:

  • about page has a direction=ltr on it; changing it to rtl works. but punctuation moves to the left on English text, which it should not. The solution would be a translation of that section in all RTL languages so the punctuation matches the text or to only align the text to the right without changing from LTR to RTL
  • the vim help is still LTR

return42 added a commit to return42/searxng that referenced this pull request Jan 14, 2022
The less grunt runner silently ignore missing files and continue with the build[1]::

    Running "less:production" (less) task
    >> Destination css/searxng.min.css not written because no source files were found.
    >> 1 stylesheet created.
    >> 1 sourcemap created.

Add filter function that calls grunt.fail() if the scr file does not exists.

[1] searxng#750 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@dalf
Copy link
Member Author

dalf commented Jan 15, 2022

The URL are displayed left to right but aligned on the right.

Google displays each URL "part" from right to left:
image

Bings displays the full URL from left to right (or is it broken?):
image

The Google version requires to modify get_pretty_url and the CSS (I'm not sure how to do that).

the vim help is still LTR

#369 needs to be fixed first

not-my-profile pushed a commit to not-my-profile/searxng that referenced this pull request Jan 15, 2022
The less grunt runner silently ignore missing files and continue with the build[1]::

    Running "less:production" (less) task
    >> Destination css/searxng.min.css not written because no source files were found.
    >> 1 stylesheet created.
    >> 1 sourcemap created.

Add filter function that calls grunt.fail() if the scr file does not exists.

[1] searxng#750 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
Copy link
Member

@return42 return42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except /preferences the RTL text orientation LGTM.

I think this PR is a huge improvement to what we have.

I left some comments with my thoughts ...

* disable declaration-empty-line-before
  https://stylelint.io/user-guide/rules/list/declaration-empty-line-before/
  this change allows to mix CSS declarations and LESS mixins without empty lines:

  #something {
    display: flex;
    .ltr-left(60rem); // no mandatory empty line before this one
  }

* disable no-invalid-position-at-import-rule
  https://stylelint.io/user-guide/rules/list/no-invalid-position-at-import-rule/

  this change allows to declare some mixins and then import another .less file:
  for example:

  .ltr-left(@offset) {
    left: @offset;
  }
  @import "style.less";
* mirror all inline SVGs so that direction SVGs display correctly on RTL
* set the bold list element in info box to RTL so the colon gets displayed on the right side
* set correct .ltr function for the left border on the search button in #q
* move text to the right in autocomplete
* move search form in lign with result article on RTL
* add the correct padding for img thumbnails in categories like music on RTL
* apply RTL to result table for map results
* align text in tables part of /preferences on RTL
* move burger menu on index page to the left on RTL
* fix positioning of drop down arrow on select boxes on RTL
* align result URL on the right (written LTR)
* align vim hotkeys help on the left since it is not translated
* image detail:
  * labels (author, format, URL, etc...) are written on the right,
    values are on the left.
  * URL are written LTR and overflow on the right
@dalf dalf changed the title [WIP] Simple: implement RTL Simple: implement RTL Jan 16, 2022
@dalf dalf changed the title Simple: implement RTL Simple theme: implement RTL Jan 16, 2022
@dalf dalf marked this pull request as ready for review January 16, 2022 17:59
Copy link
Member

@return42 return42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR is a huge improvement to what we have.

Lets merge this PR, the /preferences page can be fixed in another PR.

I have a backup if for reason it was not a good choice

from my side is no longer need for.

@dalf dalf merged commit 0c036ae into searxng:master Jan 16, 2022
@dalf dalf deleted the simple-rtl branch January 16, 2022 19:29
Copy link
Member

@mrpaulblack mrpaulblack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

(We should work on making LTR text in an RTL layout to be displayed as LTR in another PR)

@GreenLunar
Copy link
Contributor

GreenLunar commented Jan 17, 2022

Looks awesome!

Bings displays the full URL from left to right (or is it broken?):

In bing, it's reversed. URL must always be LTR and NOT affected by RTL

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.

RTL layout requires a few corrections
4 participants