Skip to content

Add Arabic locale and RTL support - #2381

Open
AliElkhedr wants to merge 4 commits into
floccusaddon:developfrom
AliElkhedr:develop
Open

AliElkhedr wants to merge 4 commits into
floccusaddon:developfrom
AliElkhedr:develop

Conversation

@AliElkhedr

Copy link
Copy Markdown

This change adds the Arabic locale bundle and enables right-to-left UI setup across the browser and native app, including RTL-aware document direction, fonts, and back-arrow icons. It also normalizes Arabic search text (diacritics, tatweel, and letter variants) in the native tree so bookmark and tag searches work consistently for Arabic content. Coverage was added for the Arabic normalization behavior in the native storage tests.

This change adds the Arabic locale bundle and enables right-to-left UI setup across the browser and native app, including RTL-aware document direction, fonts, and back-arrow icons. It also normalizes Arabic search text (diacritics, tatweel, and letter variants) in the native tree so bookmark and tag searches work consistently for Arabic content. Coverage was added for the Arabic normalization behavior in the native storage tests.
@marcelklehr

Copy link
Copy Markdown
Member

Thank you for taking the time to implement this!
Changeset looks super clean and nice!

@marcelklehr

Copy link
Copy Markdown
Member

Here's what claude says:

Likely bugs

1. Arabic search breaks for bookmarks saved before this PR (src/lib/native/NativeTreeQuery.ts:310). Search queries are now simplified before matching: vowel marks are stripped and some letters are replaced by a common form, for example ة becomes ه. Bookmarks saved before the PR have their search text stored without that change. The one-time job that fills in search text only touches empty entries, and it is already marked done for every existing account, so it won't run again. Searching ادارة therefore won't find an old bookmark titled إِدَارَةُ, even if you type the title exactly. The fix is to re-run that job once and rewrite every row, not just the empty ones.
2. Tag chips stop working for Arabic tags (src/ui/views/native/Tree.vue:728). setTagSearch and isActiveTag compare the tag with plain toLowerCase(), but the tags read back from the query are now simplified too. For a tag like وَصَفَاتٌ the two never match: the chip never shows as active, and tapping it again adds #tag a second time instead of removing it. The fix is to use normalizeSearchText in Tree.vue as well.
3. Bookmarks with Arabic text in the URL get filtered out of results (NativeTreeQuery.ts:408). The database lookup now finds a bookmark through its simplified URL. The JavaScript check that runs afterwards (matchesTerms) still compares the URL with plain toLowerCase(), so it drops the result again.

Affects all users

4. The UI font changes for everyone, not just Arabic users (src/ui/App.vue:175, plus NativeApp.vue and both HTML files). A global .v-application { font-family: … !important } replaces Vuetify's default Roboto in every language. On Android, Noto Sans Arabic now comes before Roboto in the font list, so Latin text may also render differently. This should only apply to Arabic or right-to-left pages, e.g. [dir=rtl] .v-application or :lang(ar).

Minor and cleanup

5. Right-to-left can switch on when it shouldn't (src/ui/plugins/vuetify.js:8). The initial setting turns on if any of the browser's preferred languages starts with a right-to-left code, so a user with ['en-US', 'ar'] starts in right-to-left with English text. The check also has no boundary, so arn counts as Arabic. It's corrected after the translations load, but this duplicate list could simply be dropped.
6. Some styles still use fixed left/right positions (Tree.vue:924). For example, .sync-dropdown-hint::after { left: 30px } puts the hint on the wrong side in Arabic.
7. Duplicated code: the direction setup is copy-pasted into index.js and native.js, the list of right-to-left languages exists twice, and the font list four times. These will drift apart; #5 already shows them disagreeing.
8. Back arrows are flipped one by one with $vuetify.rtl ? 'mdi-arrow-right' : 'mdi-arrow-left' in each view (Options.vue:11, …). A single CSS rule would cover every arrow, including ones added later.

@AliElkhedr

Copy link
Copy Markdown
Author

"Thanks for the thorough review! I've addressed all 8 feedback points:

Bumped search backfill to version 2 to re-index pre-existing bookmarks with normalized search text.
Handled Arabic tag chips activation/toggling via normalizeSearchText.
Fixed Arabic search in bookmark URLs with URI decoding and normalization.
Scoped Arabic font styles strictly under [dir="rtl"] and [lang^="ar"].
Extracted clean RTL locale detection (isRtlLocale) and consolidated direction setup in setupDirection(vuetify).
Fixed RTL alignment for the sync dropdown hint.
Kept standard icons in templates and handled directional flipping via CSS transform.
All Mocha tests, linting, and TypeScript checks pass cleanly."

@marcelklehr

Copy link
Copy Markdown
Member

Woop woop!

Can you drop _locales/ar/messages.json - we have it in develop from Transifex by now. Please use Transifex to fill those translations.

Additionally, the search backfill can overwrite fresh search text (NativeTreeQuery.ts:261). It now reads every row, computes search_text in JavaScript, and writes it back later in a separate batch. If a sync saves a renamed bookmark in between, the backfill puts back text built from the old title. The bookmark then can't be found by its new title until it is edited again, because search_backfilled=2 stops any further rebuild. A guard in the UPDATE (only write if search_text still holds the value read) would fix it.

Also, percent-encoded URLs are no longer searchable (NativeTreeStore.ts:69). Both the stored search text and the matching now use the decodeURI()'d URL, so a search for a%20b, or for a pasted encoded URL fragment, stops finding the bookmark.

v-if="items.length > 1"
:items="items"
:style="{...(inItem && { marginLeft: '-1.5em', marginTop: 0, marginBottom: 0, paddingTop: 0, paddingBottom: 0 })}">
:style="{...(inItem && { [($vuetify && $vuetify.rtl) ? 'marginRight' : 'marginLeft']: '-1.5em', marginTop: 0, marginBottom: 0, paddingTop: 0, paddingBottom: 0 })}">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we use marginInlineStart?

Comment thread src/ui/App.vue Outdated

<style>
[dir="rtl"] .v-application,
[lang^="ar"] .v-application {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we drop ar here? Seems redundant. Alternatively, we should list all rtl langs here, I think :)

…ale, breadcrumb inline margin

Index both encoded and decoded bookmark URLs, and avoid overwriting newer search text during backfill. Use text direction for RTL font styling and logical breadcrumb spacing; remove the Arabic locale file.
@AliElkhedr

Copy link
Copy Markdown
Author

Thanks for the follow-up review! I've addressed all the points in the latest commit:

  1. Dropped _locales/ar/messages.json: Removed from the PR so translations stay managed through Transifex. I have also already completed and uploaded the full Arabic translation directly to the Transifex project!
  2. Backfill Concurrency Guard: Added AND search_text IS ? in the update queries in NativeTreeQuery to guard against overwriting concurrent bookmark/folder updates.
  3. Encoded URL Search: Both raw (encoded) and decoded URLs are now stored in bookmarkSearchText and checked in matchesTerms, ensuring searches for encoded fragments (like %20) work alongside decoded Arabic text. Added a test for this as well.
  4. Breadcrumbs: Switched to the CSS logical property marginInlineStart: '-1.5em'.
  5. Font Styles: Removed [lang^="ar"] and kept it strictly scoped to [dir="rtl"] .v-application.

All tests (233 passing), linting, and type checks continue to pass cleanly.

This branch has not been deployed

No deployments
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.

2 participants