jellyfin-web icon indicating copy to clipboard operation
jellyfin-web copied to clipboard

Add Initial support for RTL layouts

Open hadicharara opened this issue 2 years ago • 4 comments

Motivation

RTL languages such as Arabic require RTL layouts for a proper user experience. While usable, the current state of affairs is sub-par for users of Jellyfin in RTL languages.

Examples Screenshot 2022-07-02 at 15-56-37 Jellyfin Screenshot 2022-07-02 at 16-00-06 Jellyfin Screenshot 2022-07-02 at 16-00-45 Jellyfin

Philosophy

  • My changes are meant to be as unobtrusive as possible to core Jellyfin contributors. All CSS changes made will only affect users of RTL languages. When modifying the css, Jellyfin needn't worry about RTL layouts (although it is very easy to accomodate for them).
  • Jellyfin already has language settings, independent of the browser's settings. I aimed to respect the Jellyfin user's language and locale settings.

Changes

  • When globalize detects a RTL language, it injects the dir="rtl" attribute to the HTML body. Most of the work is done by this simple fix.
  • CSS changes are made primarily with the inline and block variant attributes for inser, margin and padding, else by checking the dir attribute.
  • Javascript changes has been made to fix the emby-slider component.
  • Some changes made to support alternative numeral systems (my motivation was eastern arabic numerals). This is done by using toLocaleString() and similar functions.
  • rtl.scss file added to flip horizontally directional icons.

Resuts Screenshot 2022-07-02 at 16-13-36 Jellyfin Screenshot 2022-07-02 at 16-14-38 Jellyfin Screenshot 2022-07-02 at 16-16-04 Jellyfin Screenshot 2022-07-02 at 16-17-07 Jellyfin

There are probably many things that could be improved with my changes (probably some things with the usage of toLocaleString() could be improved). I welcome any feedback!

TODO: testing on TV and mobile layouts. Thoroughly checking the application for inconsistencies in RTL layouts. No changes should be visible to LTR layout users, though this should be checked.

hadicharara avatar Jul 02 '22 20:07 hadicharara

Also the mainDrawer still opens on the left. This is to be fixed (probably by changing the lib file itself).

hadicharara avatar Jul 02 '22 20:07 hadicharara

Jellyfin has been tested in desktop, mobile and tv layouts, both in LTR and RTL layouts. All bugs I could fine have been crushed (except the fact that the dashboard drawer animation comes from the left, but that's fine). It is currently 100% usable and I have been using it for days. I await feedback!

hadicharara avatar Jul 08 '22 17:07 hadicharara

Information for reviewers: Some elements have been hardcoded as ltr:

  • The loading spinner because it should look the same in ltr and rtl.
  • The video player is LTR otherwise the subtitle canvas get messed up.
  • File paths are always "ltr".
  • Player controls are "ltr" because it doesn't represent directionality rather the direction of a tape recording.

Testers in any platform (desktop, TV, mobile) and in any direction are welcome.

hadicharara avatar Jul 08 '22 17:07 hadicharara

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarcloud[bot] avatar Jul 14 '22 22:07 sonarcloud[bot]

hi you can please add the Hebrew language to the display from right to left as in the Arabic language

CAPRICORNX10X avatar Oct 01 '22 13:10 CAPRICORNX10X

This should have added support for all RTL languages in Jellyfin. However, this PR should be closed for a lack of interest, since it was't properly reviewed.

hadicharara avatar Oct 01 '22 16:10 hadicharara

Oh we are definitely interested in landing this! The problem is it's basically only me reviewing larger changes to web right now and I've been focusing on getting the new website going. That said I'm back around now working through reviews so if you could rebase this I'll work on reviewing it in the next few days. 💜

thornbill avatar Oct 01 '22 17:10 thornbill

Alright no problem! It should be rebased soon.

hadicharara avatar Oct 01 '22 19:10 hadicharara

Just merged latest changes from master and tested.

hadicharara avatar Oct 01 '22 21:10 hadicharara

Looks like there are several (minor) lint issues now:

src/assets/css/librarybrowser.scss
 540:5  ✖  Unexpected empty line before closing brace                                                                                                     block-closing-brace-empty-line-before
 549:5  ✖  Expected empty line before rule                                                                                                                rule-empty-line-before               
 811:5  ✖  Expected selector "[dir="rtl"] .detailPagePrimaryContainer" to come before selector ".layout-desktop [dir="ltr"] .detailPagePrimaryContainer"  no-descending-specificity            
 811:5  ✖  Expected selector "[dir="rtl"] .detailPagePrimaryContainer" to come before selector ".layout-tv [dir="ltr"] .detailPagePrimaryContainer"       no-descending-specificity            

src/components/actionSheet/actionSheet.scss
 51:5  ✖  Unexpected empty line before closing brace  block-closing-brace-empty-line-before

src/components/alphaPicker/style.scss
 120:51  ✖  Expected a trailing semicolon  declaration-block-trailing-semicolon

src/components/cardbuilder/card.scss
 305:5  ✖  Expected empty line before rule             rule-empty-line-before               
 308:5  ✖  Unexpected empty line before closing brace  block-closing-brace-empty-line-before
 458:5  ✖  Unexpected empty line before closing brace  block-closing-brace-empty-line-before

src/components/imageUploader/style.scss
 15:17  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after

src/components/imageeditor/imageeditor.scss
 17:25  ✖  Expected a trailing semicolon  declaration-block-trailing-semicolon
 23:22  ✖  Expected a leading zero        number-leading-zero                 
 27:23  ✖  Expected a leading zero        number-leading-zero                 

src/components/libraryoptionseditor/style.scss
 3:21  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after
 7:22  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after

src/components/listview/listview.scss
 107:1  ✖  Expected selector ".listItem" to come before selector "[dir="ltr"] .listItem"  no-descending-specificity
 107:1  ✖  Expected selector ".listItem" to come before selector "[dir="rtl"] .listItem"  no-descending-specificity

src/components/mediaLibraryCreator/style.scss
 13:22  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after
 13:22  ✖  Expected a leading zero                                         number-leading-zero          
 17:23  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after
 17:23  ✖  Expected a leading zero                                         number-leading-zero          

src/components/mediaLibraryEditor/style.scss
 13:22  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after
 13:22  ✖  Expected a leading zero                                         number-leading-zero          
 17:23  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after
 17:23  ✖  Expected a leading zero                                         number-leading-zero          

src/components/metadataEditor/style.scss
 13:21  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after
 17:22  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after

src/components/multiSelect/multiSelect.scss
 45:17  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after

src/components/playerstats/playerstats.scss
 59:5  ✖  Unexpected empty line before closing brace  block-closing-brace-empty-line-before

src/components/subtitleuploader/style.scss
 14:17  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after
 17:1   ✖  Expected no more than 1 empty line                              max-empty-lines              

src/components/tvproviders/style.scss
 3:16  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after
 3:22  ✖  Expected a leading zero                                         number-leading-zero          
 7:16  ✖  Expected single space after ":" with a single-line declaration  declaration-colon-space-after
 7:18  ✖  Expected a leading zero                                         number-leading-zero          

src/elements/emby-input/emby-input.scss
 55:23  ✖  Expected a trailing semicolon                                                                                                               declaration-block-trailing-semicolon
 59:1   ✖  Expected selector ".inlineForm .inputContainer:last-child" to come before selector "[dir="ltr"] .inlineForm .inputContainer:first-child"    no-descending-specificity           
 59:1   ✖  Expected selector ".inlineForm .inputContainer:last-child" to come before selector "[dir="rtl"] .inlineForm .inputContainer:first-child"    no-descending-specificity           
 60:1   ✖  Expected selector ".inlineForm .selectContainer:last-child" to come before selector "[dir="ltr"] .inlineForm .selectContainer:first-child"  no-descending-specificity           
 60:1   ✖  Expected selector ".inlineForm .selectContainer:last-child" to come before selector "[dir="rtl"] .inlineForm .selectContainer:first-child"  no-descending-specificity           

src/elements/emby-scroller/emby-scroller.scss
 25:1  ✖  Expected selector ".servers > .card > .cardBox" to come before selector "[dir="ltr"] .itemsContainer > .card > .cardBox"  no-descending-specificity
 25:1  ✖  Expected selector ".servers > .card > .cardBox" to come before selector "[dir="rtl"] .itemsContainer > .card > .cardBox"  no-descending-specificity

42 problems (42 errors, 0 warnings)

thornbill avatar Oct 03 '22 14:10 thornbill

Sorry. Should be good now.

hadicharara avatar Oct 03 '22 17:10 hadicharara

Fixed all the issues you pointed out. Sorry for the delay, I was a bit busy.

hadicharara avatar Oct 12 '22 12:10 hadicharara

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarcloud[bot] avatar Oct 14 '22 21:10 sonarcloud[bot]