kiwix / kiwix-desktop

Kiwix for Windows and GNU/Linux desktops
https://download.kiwix.org/release/kiwix-desktop/
GNU General Public License v3.0
747 stars 98 forks source link

Fix Search Bar Cursor Alignment in RTL #1187

Closed ShaopengLin closed 2 weeks ago

ShaopengLin commented 1 month ago

Fix #594

The search bar cursor when no text is present now aligns to the right in RTL. See comment in the issue for the previous faulty behavior.

I tested by using -reverse option as well as changing Kiwix Desktop Locale to RTL languages.

kelson42 commented 4 weeks ago

I have rebased, and for some reason now kiwix-desktop does not find the libkiwix anymore?!?!?:

$ ./kiwix-desktop
./kiwix-desktop: error while loading shared libraries: libkiwix.so.14: cannot open shared object file: No such file or directory

Some kind of problem with the versioning of the libkiwix14... but can not identify which one

kelson42 commented 3 weeks ago

@ShaopengLin How should I test this, considering that you don't have implemented -reverse? Can you please put screenshots proving that everything is displayed in the right order + clear procedure how to test the PR.

ShaopengLin commented 3 weeks ago

@kelson42 1. when search bar text is LTR but in reverse (RTL) mode, we see text not aligning with the suggestions. Screenshot from 2024-09-03 02-27-22 (1) Similarly when text is RTL but in LTR mode: Screenshot from 2024-09-03 02-30-26 (1) The first change should align them to their respective positions: Screenshot from 2024-09-03 02-41-44 (1) Screenshot from 2024-09-03 02-41-21 (1)


2. When placeholder text "search" is in RTL language, it is aligned to the opposite position of suggestions: In RTL mode: Screenshot from 2024-09-03 02-54-54 In LTR Mode: Screenshot from 2024-09-03 02-54-42 You can test this by changing the locale of Kiwix Desktop. The second change should align them: Screenshot from 2024-09-03 02-59-47 Screenshot from 2024-09-03 02-59-56


3. The displayed text for fulltext-search on the search bar does not match what it is in the suggestions. LTR search text in RTL Locale (Not reverse mode):Screenshot from 2024-09-03 05-35-00 RTL search text in LTR Locale (Not reverse mode): Screenshot from 2024-09-03 03-31-58

In addition, if you are in reverse mode for both RTL and LTR locale, the '(Fulltext search)' can get inserted in between:

  1. In LTR reverse mode, Type ه
  2. copy paste s
  3. you see s (Fulltext search) ه Screenshot from 2024-09-03 03-42-26
  4. In RTL reverse mode, Type s
  5. copy paste ه
  6. you see sه (بحث في النص الكامل) Screenshot from 2024-09-03 03-43-59

This third change should align them properly to the text direction: Mismatch Resolved: Screenshot from 2024-09-03 05-36-50 Screenshot from 2024-09-03 05-40-16 Wrong placeholder insertion Resolved: Screenshot from 2024-09-03 05-44-45 Screenshot from 2024-09-03 05-42-00

kelson42 commented 3 weeks ago

@ShaopengLin This is a PR, this means the solution: never describe a bug in a PR, this is super confusing. The bug is described in the issue and should be clear there.

You talk about a "reverse mode" What is that? This is not described in the PR description. The "-reverse" option was made as a development tool in previous PR to avoid to have to change the local in Ubuntu which is a cumbersome operation.... which BTW I have no clue why you refuse to implement.... but anyway therefore not available here. So what is your "reverse mode".

You don't have to explain how it behaves writting RTL language in a UI which is LTR. First of all I know this is weird but this is not something you have any impact anyway... this is not something your PR (can) handle. Your PR description should focus on your work, only that.

ShaopengLin commented 3 weeks ago

@kelson42 Sorry I wasn't using the terminalogy. The reverse mode I mentioned is using the "-reverse" option. I have moved the bug report to the issue.

kelson42 commented 3 weeks ago

Exactly the same as before, to me your PR "does nothing". Just starting:

LANG=AR ./kiwix-desktop -reverse

and getting: image

... nothing is inverted!

ShaopengLin commented 3 weeks ago

@kelson42 -reverse and the Locale setting shouldn't be done at the same time. Setting Locale by itself already inverts the app.

kelson42 commented 3 weeks ago

@ShaopengLin Looks better indeed! At a first look I don't see anything not working. @veloman-yunkan Could you please have a look yourself?