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

fix(indicate typos): letters not displaying correctly in RTL languages or with ligatures when set to below (NadAlaba) #5113

Merged
merged 10 commits into from
Mar 25, 2024

Conversation

NadAlaba
Copy link
Contributor

@NadAlaba NadAlaba commented Feb 23, 2024

Description

  1. Moved <hint> elements from <letter .incorrect> to <.word>, because hints were messing the joining of letters in languages with cursive attachments "withLigatures". Hint characters, that correspond to adjacent incorrect letters, now join together as a word when there is an overlap between them, which can happen in non monospaced fonts regardless if the current language has "ligatures" or not (In test-ui.ts: New functions createHintsHtml() and async function joinOverlappingHints(), and changes to updateWordElement()).
  2. Recalculate hints position whenever font size or font family is changed (In test-ui.ts: New async function updateHintsPosition())
  3. Made updateWordElement() async in order to call await joinOverlappingHints();, and made all calls to this function void updateWordWlement(); (In input-controller.ts and test-ui.ts)
  4. Styled <.word> as position: relative instead of .word letter.incorrect {position: relative}, in order to be able to position <hint>s absolutely relative to the corresponding <.word> (In styles/test.scss).
  5. Corrected the position of the caret, because after making <.word> relative, <letter>s were giving offsetLeft and offsetTop relative to the <.word> instead of relative to <#wordsWrapper>, so had to add the offset position of <.word .active> to the position of the caret. (In caret.ts: Changes to updatePosition()).
  6. Same changes as 4 above to the pace caret (In pace-caret.ts: Changes to update()).
  7. Corrected the position of the starting point of the pace caret in RTL languages, because it was starting after the first letter in each word, and had to make resetCaretPosition() and update() async in order to check if current language is RTL (to be able to call await Misc.getCurrentLanguage();), and made the call to update() void update() in function start() {} (In pace-caret.ts: Changes to resetCaretPosition(), update(), and start()).

Checks

  • Adding quotes?
    • Make sure to include translations for the quotes in the description (or another comment) so we can verify their content.
  • Adding a language or a theme?
    • If is a language, did you edit _list.json, _groups.json and add languages.json?
    • If is a theme, did you add the theme.css?
      • Also please add a screenshot of the theme, it would be extra awesome if you do so!
  • Check if any open issues are related to this PR; if so, be sure to tag them below.
  • Make sure the PR title follows the Conventional Commits standard. (https://www.conventionalcommits.org for more info)
  • Make sure to include your GitHub username inside parentheses at the end of the PR title

@monkeytypegeorge monkeytypegeorge added the frontend User interface or web stuff label Feb 23, 2024
@NadAlaba
Copy link
Contributor Author

P.S. There has to be better ways of checking current language in sync functions

@Miodec
Copy link
Member

Miodec commented Feb 26, 2024

If you only type incorrect letters, the hints jump right after each keypress.

@Miodec Miodec added the waiting for update Pull requests or issues that require changes/comments before continuing label Feb 26, 2024
@NadAlaba
Copy link
Contributor Author

If you only type incorrect letters, the hints jump right after each keypress.

Couldn't reproduce that!
Screenshot 2024-02-26 161459
Perhaps you're talking about the joining behavior of hint letters, which clusters (joins) consecutive hint-letters together, and centers the hint block with the incorrect letters block, this may cause the hint-letters block to be slightly longer or shorter than the incorrect letters block, but it will ensure that hints don't overlap each other in non monospaced fonts.

@Miodec
Copy link
Member

Miodec commented Feb 26, 2024

Please test with english.

@NadAlaba
Copy link
Contributor Author

Please test with english.

Done! Edited my first comment to reflect chagnes.

@NadAlaba NadAlaba requested a review from Miodec February 28, 2024 07:00
Also, make the functions resetCaretPosition() and update() async, in order to
check for current language inside them with await Misc.getCurrentLanguage().
Also, make the function updateWordElement() async, in order to check for
current language inside it with await Misc.getCurrentLanguage(), and
make all calls to this function void updateWordElement() in
input-controller.ts and test-ui.ts.
instead of triggering hint clustering (joining) as a word only in
languages that have 'ligatures' = true, hints are now joined only if there is
overlap between hint characters.
@NadAlaba
Copy link
Contributor Author

NadAlaba commented Mar 5, 2024

@Miodec Any feedback?

@Miodec
Copy link
Member

Miodec commented Mar 5, 2024

Will check soon.

@Miodec Miodec added waiting for review Pull requests that require a review before continuing and removed waiting for update Pull requests or issues that require changes/comments before continuing labels Mar 11, 2024
@Miodec
Copy link
Member

Miodec commented Mar 11, 2024

Seems to be good, all though when changing font size, could you recalculate the positions?
image

@Miodec Miodec added waiting for update Pull requests or issues that require changes/comments before continuing and removed waiting for review Pull requests that require a review before continuing labels Mar 11, 2024
@NadAlaba NadAlaba marked this pull request as draft March 13, 2024 23:23
@NadAlaba
Copy link
Contributor Author

Seems to be good, all though when changing font size, could you recalculate the positions?

Done!

  1. Recalculate hints position whenever font size or font family is changed (In test-ui.ts: New async function updateHintsPosition())

@NadAlaba NadAlaba marked this pull request as ready for review March 13, 2024 23:33
@Miodec Miodec changed the title fix: Indicate typos below disjoining letters in languages with ligatures (NadAlaba) fix(indicate typos): disjoining letters in RTL languages or with ligatures when set to below (NadAlaba) Mar 25, 2024
@Miodec Miodec changed the title fix(indicate typos): disjoining letters in RTL languages or with ligatures when set to below (NadAlaba) fix(indicate typos): letters not displaying correctly in RTL languages or with ligatures when set to below (NadAlaba) Mar 25, 2024
@Miodec Miodec merged commit c20964d into monkeytypegame:master Mar 25, 2024
9 checks passed
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 update Pull requests or issues that require changes/comments before continuing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants