Skip to content

impr(tape mode): support RTL languages (@NadAlaba) #5748

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

NadAlaba
Copy link
Contributor

@NadAlaba NadAlaba commented Aug 8, 2024

Description

  1. Support RTL in tape mode:

    • In scrollTape(): flip the sign of #words.margin-left and add .word.margin-right to center first letter in RTL.
    • In Caret.getTargetPositionLeft(): flip the direction of tapeMargin in RTL.
    • Remove restriction on RTL tape mode from test-logic.ts.
  2. Support zero-width characters in tape mode:

    • Subtract the width of the last letter that has a positive width if the current letter has a zero width (e.g, diacritics). This is needed when calculation is based on letter widths instead of letter position, which is done in caret.ts when tape=word, and in scrollTape() when tape=letter.
  3. Remove the width change of #words in tape mode to 200vw because it's not needed anymore now that we're using white-space: nowrap:

    • Also adjust the limit of .afterNewline.margin-left to be 3 times the new width of #words which is now equal to #wordsWrapper.width by default.
  4. Make .word.height in .withLigature langs similar to their height in english:

    • Imitate the appearance and behavior of inline-block <letter>s in .withLigatures lanuages. These languages make the display of <letter> elements inline in order to allow the joining of letters. However, this causes <letter>'s border-bottom to be ignored, which changes .word height, so we add a padding-bottom to the .word in that case.
    • Also, inline <letter>s overflow the #wordWrapper without wrapping (e.g, when maxLineWidth = 20ch and we type 30 letters), so we add the property overflow-wrap: anywhere, but we don't allow .hints to inherit this property.
    • P.S, it is necessary that all .words have the same height (with and without ligatures), because we now set the height of .beforeNewlines in css, and we depend on these elements to have the same height as .words so that the user won't feel a vertical shift in lines in tape mode.
  5. Animate turning off tape mode in updateWordsMargin() if SmoothLineScroller is on.

  6. Block removing words at the first call of scrollTape():

    • Because the inline style of #words.margin-left may be negative when restarting the test, making scrollTape() start when the first word is overflown to the left, which makes scrollTape() remove that word (this bug affects LTR and RTL langs).

closes #3923

@monkeytypegeorge monkeytypegeorge added the frontend User interface or web stuff label Aug 8, 2024
Copy link
Contributor

This PR is stale. Please trigger a re-run of the PR check action.

@github-actions github-actions bot added the Stale Has not been updated in a while label Aug 16, 2024
@Miodec
Copy link
Member

Miodec commented Aug 23, 2024

Weird issue i ran into when testing this (sometimes words are slow to appear on the right)

Screen.Recording.2024-08-23.at.14.49.04.mov

@Miodec Miodec added the waiting for update Pull requests or issues that require changes/comments before continuing label Aug 23, 2024
@github-actions github-actions bot removed the Stale Has not been updated in a while label Aug 23, 2024
@NadAlaba
Copy link
Contributor Author

Weird issue i ran into when testing this (sometimes words are slow to appear on the right)

Resolved!

Please merge #5824 before this to perform final test on this one.

Copy link
Contributor

github-actions bot commented Sep 2, 2024

This PR is stale. Please trigger a re-run of the PR check action.

@github-actions github-actions bot added the Stale Has not been updated in a while label Sep 2, 2024
@NadAlaba NadAlaba marked this pull request as draft September 10, 2024 18:32
@github-actions github-actions bot removed the Stale Has not been updated in a while label Sep 10, 2024
@github-actions github-actions bot removed the waiting for update Pull requests or issues that require changes/comments before continuing label Sep 10, 2024
@NadAlaba NadAlaba marked this pull request as ready for review September 10, 2024 23:01
@github-actions github-actions bot added the waiting for review Pull requests that require a review before continuing label Sep 10, 2024
@NadAlaba
Copy link
Contributor Author

This is built on top of #5868, so it'd be easier to review that PR first

@NadAlaba NadAlaba force-pushed the tapeRTL branch 9 times, most recently from 3c4b9b1 to 7b14235 Compare September 18, 2024 20:42
@NadAlaba NadAlaba marked this pull request as draft September 26, 2024 16:05
@github-actions github-actions bot removed the waiting for review Pull requests that require a review before continuing label Sep 26, 2024
@NadAlaba NadAlaba force-pushed the tapeRTL branch 2 times, most recently from 35bbcbf to 211a24a Compare September 26, 2024 20:26
@Miodec Miodec reopened this Nov 4, 2024
@github-actions github-actions bot removed the Stale Has not been updated in a while label Nov 9, 2024
Copy link
Contributor

This PR is stale. Please trigger a re-run of the PR check action.

@github-actions github-actions bot added the Stale Has not been updated in a while label Nov 19, 2024
@github-actions github-actions bot closed this Dec 8, 2024
@Miodec Miodec reopened this Dec 9, 2024
@github-actions github-actions bot removed the Stale Has not been updated in a while label Dec 10, 2024
@NadAlaba NadAlaba marked this pull request as draft March 5, 2025 15:41
@github-actions github-actions bot removed the waiting for review Pull requests that require a review before continuing label Mar 5, 2025
@NadAlaba NadAlaba force-pushed the tapeRTL branch 5 times, most recently from 589d113 to 5bcd1e5 Compare April 5, 2025 12:10
@NadAlaba NadAlaba force-pushed the tapeRTL branch 2 times, most recently from 40fda7e to 127c3c9 Compare April 7, 2025 18:01
@NadAlaba NadAlaba force-pushed the tapeRTL branch 2 times, most recently from e3ccac2 to c65877f Compare April 15, 2025 15:51
@NadAlaba NadAlaba marked this pull request as ready for review April 15, 2025 16:08
@github-actions github-actions bot added the waiting for review Pull requests that require a review before continuing label Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend User interface or web stuff waiting for review Pull requests that require a review before continuing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update tape mode to support right to left languages
3 participants