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

Implement right-to-left interface #3970

Merged
merged 69 commits into from
Sep 9, 2023

Conversation

kommunarr
Copy link
Collaborator

@kommunarr kommunarr commented Aug 29, 2023

Implement right-to-left interface

Co-authored by @ChunkyProgrammer

Pull Request Type

  • Feature Implementation

Related issue

closes #1550

Description

At a high level, this PR updates the interface of FreeTube to accommodate the proper patterns for right-to-left languages throughout the entire app. This PR looks quite large, and it is, but >80-85% of the changes are migrating CSS properties to their 1:1 directionality-agnostic equivalents.

Implementation-wise:

  • Doesn't touch VideoJS.css
  • Replaces margin-left / -right with margin-inline-start / -end & margin-top / -bottom with margin-block-start / -end
  • Replaces padding-left / -right with padding-inline-start / end & padding-top / -bottom with padding-block-start / -end
  • Replaces left/right with inset-inline-start/-end & top/bottom with inset-block-start/-end
  • Replaces border-left*/border-right* with border-inset-start/-end & border-top*/border-bottom* with border-block-start/-end
  • Replaces text-align: left/right with text-align: start/end
  • Custom: replaces float: left/right with two CSS variables (because float: inline-start/-end is still experimental in Chrome)
  • Custom: modifies transform() x values to conditionally be inverted by multiplying them with a CSS variable of 1 or -1 based on language directionality (090f392)
  • Custom styling was needed/added in one place (ft-input.css to fix an emergent issue with the search bar magnifying-glass icon positioning on languages with right-to-left directionality
  • Fixes settings switches not being aligned when the locale has right-to-left directionality
  • Horizontally flips icon direction when directionality is RTL with two exceptions (the magnifying-glass on search bars and circle-question on the About route)
  • Updates arrow keys when the current locale has right-to-left directionality such that left arrow is the "history forward" arrow, and right arrow is the "history backward" arrow (as is standard in RTL browsers)
  • Refactors the arrow key logic to use class binding (to resolve emergent bug with history arrows showing as disabled when changing the locale to one with different directionality). It's also better practice than whatever the original implementer was doing (me, in 2021, to be clear)

Screenshots

Screenshot_20230829_043452
Screenshot_20230829_043506
Screenshot_20230829_043545
Screenshot_20230829_043955
Screenshot_20230829_044057
Screenshot_20230829_044113
Screenshot_20230829_044321
Screenshot_20230829_083913

Testing

Test Plan

  • Under General Settings, set locale to one with right-to-left directionality (e.g., Arabic / العربية )
    • Check that sliders in, e.g., Player Settings, are right-to-left
    • Check that switches/toggles in, e.g., Player Settings, are right-to-left and are aligned right
    • Check that tooltips in, e.g., Player Settings, open correctly
    • Check that arrow navigation works correctly by going forward one route, then going back (left = forward, right = back)
    • Check that all icons are horizontally flipped with the exception of the magnifying-glass on search bars and circle-question on the About route
    • Check that labels left-aligned in ltr languages are right-aligned (e.g., side-nav labels, video titles, search results, ft-input placeholders, comments sections, the video count container inside each playlist thumbnail in the Channel Playlists tab, the labels in the profile sections, polls in the Channel Community tab)
    • Check that rtl directionality and styling is maintained after Ctrl+R
    • (Optional) Prepend true || to v-if statements on lines 17 and 21 of App.vue to show & validate updates banner is rtl

Desktop

  • OS: OpenSUSE Tumbleweed
  • OS Version: 2023xxxx
  • FreeTube version: 0.19.0

Future Action Items

Add linter requiring use of directionality-neutral CSS properties

To maintain a usable right-to-left interface, we must ensure that the directionality-agnostic property names listed above are used in all cases going forward (with maybe the sole exception of videoJS.css).

Ongoing Localization Problem

We currently use string concatenation of data and an i18n label in many places in the code (e.g., "v0.19.0 Beta" on the About page, "5,000 views" on video lists. This is not the way to do this, because many languages order those values differently. This will need to be addressed in a separate PR given that this is more of a localization problem, but I thought it would mention it specifically because languages with right-to-left directionality are most notably affected.

Enhancement: Switch left-to-right direction in ft-inputs and their search results based on whether the text typed is in an ltr or rtl language.

Support for vertical writing modes

This PR actually does much of the heavy lifting for accommodating vertical writing modes. The side nav bar, top nav bar, and many controls' styling will have to be reworked for this to be a reality, but these directionality-agnostic properties aren't just useful for rtl support.

To RTL language users

let us know if you are having any issues with directionality in the video player or any other part of FreeTube!

…lacing padding-left and padding-right with padding-inline-start and padding-inline-end
…ing margin-left and margin-right with margin-inline-start and margin-inline-end
This commit also changes border-left to border-inline-start, and border-right to border-inline-end.
…lock-start, and inset-block-end

This commit also replaces border-top and border-bottom with border-block-start and border-block-end.
To the fullest of my understanding, it is expected for the left navigation in rtl-supporting browsers to be for forward navigation, and the right one to be for backward navigation.
…ems: start

I have validated this for all settings tabs; the justify-content: start did nothing in any language, left-to-right or right-to-left. Replacing it with align-items: start aligned all menu switch items by their switches, not by their labels. This makes for a much more uniform settings section for trl languages.
…hanging language to one with different directionality

Given that which icon is displayed for which history arrow is now dynamic based on the user's directionality, changing of the icon resets the font-awesome-icon state and thus re-adds the base disabled class to both arrows. This means that changing your language to one that has a different directionality was falsely setting the arrows to their disabled state (until the route is changed, after which the history icons will be fully back to normal). This commmit refactors the history icon setting logic to use class binding to two booleans in the top-nav component's data rather than adding and removing the disabled classes directly to the arrow elements' classLists, thus cleaning up the implementation and fixing the bug.
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Aug 29, 2023
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) August 29, 2023 07:42
…f locale value from store

Kudos to absidue for pointing out this existing function for representing this, and that it grabs directly from the i18n object. This means that FreeTube will display the proper rtl interface if one is the user's system language, where it did not before.
Leverage the icons being flipped rather than manually setting the classes to their opposites when the directionality is RTL.
@absidue
Copy link
Member

absidue commented Sep 4, 2023

Added the problem with using concatenation instead of placeholders to the Development Chores board: https://github.com/FreeTubeApp/FreeTube/projects/10. We already use placeholders in some places (search for { in the en-US.yaml file), but not in some notable places, so we should change to placeholders everywhere.

kommunarr and others added 2 commits September 5, 2023 01:24
Co-authored-by: absidue <48293849+absidue@users.noreply.github.com>
Co-authored-by: absidue <48293849+absidue@users.noreply.github.com>
@kommunarr kommunarr requested a review from absidue September 6, 2023 02:06
Co-authored-by: PikachuEXE <pikachuexe@gmail.com>
Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

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

Messes up any LTR text e.g. the video description where link rendering breaks, but I don't think that's solvable unless we embed translation tool level language detection, to pick the direction based on the content.

@FreeTubeBot FreeTubeBot merged commit 6f2a535 into FreeTubeApp:development Sep 9, 2023
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Sep 9, 2023
@kommunarr
Copy link
Collaborator Author

kommunarr commented Sep 10, 2023

@atpersian @yarons @rex07 These changes are now present in the latest nightly build. Here is more information on how to use nightly builds. Please feel welcome to update here with any comments or concerns! You are also welcome to compare & contrast with YouTube's current RTL support.

@atpersian
Copy link
Contributor

Dear Dude
how can i use nightly build?? is there any guide?

@kommunarr
Copy link
Collaborator Author

Dear Dude how can i use nightly build?? is there any guide?

Here you go: https://docs.freetubeapp.io/development/nightly-builds

PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Sep 11, 2023
* development: (46 commits)
  Translated using Weblate (Hebrew)
  Translated using Weblate (Hebrew)
  Translated using Weblate (Chinese (Simplified))
  Add subscribed icon to comments (FreeTubeApp#4007)
  Translated using Weblate (Icelandic)
  Translated using Weblate (Czech)
  Translated using Weblate (Bulgarian)
  Translated using Weblate (Polish)
  Translated using Weblate (Arabic)
  Translated using Weblate (Greek)
  Translated using Weblate (Italian)
  Translated using Weblate (Chinese (Simplified))
  Translated using Weblate (Hungarian)
  Translated using Weblate (Portuguese (Brazil))
  Translated using Weblate (Ukrainian)
  Translated using Weblate (Spanish)
  Translated using Weblate (Hebrew)
  Translated using Weblate (Turkish)
  Search: Add hashtags to results (FreeTubeApp#3780)
  Implement right-to-left interface (FreeTubeApp#3970)
  ...

# Conflicts:
#	src/renderer/components/ft-element-list/ft-element-list.vue
#	src/renderer/components/ft-prompt/ft-prompt.css
#	src/renderer/components/ft-toast/ft-toast.css
#	src/renderer/scss-partials/_ft-list-item.scss
#	src/renderer/views/Playlist/Playlist.scss
@atpersian
Copy link
Contributor

i check Artifacts file , change some translate for better understanding contents. (language name and ..etc) . thanks for your response .
B R

@yarons
Copy link
Contributor

yarons commented Sep 11, 2023

No glitches yet, looking pretty good.
I fixed some translations.

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.

Please add RTL interface
8 participants