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.

Sign up for GitHub

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

Jump to bottom

Highlight leading and trailing spaces, and double/multiple spaces in the middle #1500

Merged
merged 15 commits into from
Apr 19, 2023
Merged

Highlight leading and trailing spaces, and double/multiple spaces in the middle #1500

merged 15 commits into from
Apr 19, 2023

Conversation

pedro-mendonca
Copy link
Member

@pedro-mendonca pedro-mendonca commented Oct 5, 2022 •
edited
Loading

What?

Highlight the leading and trailing spaces both in the preview and editor rows
Also highlight double and multiple spaces in the middle.

This PR also adds a missing unit test for the invisible characters markup added on prepare_original() for line breaks and tabs.

Why?

Allows the translator to see the spaces at the beginning or the end of an original/translation.
Allows authors and translators to see double and multiple spaces in the middle of strings.

How?

Wrap the spaces in a <span> with invisible-spaces class, which adds a grey color.

Testing Instructions

  1. Have some original string with spaces in the beginning or end, or with double spaces in the middle
  2. Open the translation set where the string appears, see the highlighted spaces in grey in the preview row, both in original and preview (since 7a31b0f)
  3. Open the translation string editor and see the highlighted spaces in grey, both in the original and translation.

imagem

imagem

Fixes #863.

@pedro-mendonca pedro-mendonca requested a review from amieiro October 5, 2022 13:43
@pedro-mendonca
Copy link
Member Author

pedro-mendonca commented Oct 5, 2022 •
edited
Loading

@amieiro currently the Translation markup is added to editor <blockquote> through prepare_original(), which is probably wrong because also searches for glossary etc.
To add it to both the translation bloquote and preview row, maybe is better to use another function (prepare_translation_preview()) and add the highlight to it.

@pedro-mendonca
Copy link
Member Author

pedro-mendonca commented Oct 5, 2022 •
edited
Loading

Using the prepare_original() in the translation of the preview row, as it's used for the current translation blockquote above the textarea, adds the markup. This function still uncessasry searches for glossary entries, but none will be found.

imagem

@pedro-mendonca
Copy link
Member Author

Added support for strings with multiple lines:
imagem

@amieiro amieiro mentioned this pull request Oct 10, 2022
Closed
@pedro-mendonca
Copy link
Member Author

Adding another highlight: double/multiple spaces between words.

imagem

Changing the CSS to a darker color with opacity 50% so it works better on different backgrounds of the string status:

imagem

@pedro-mendonca pedro-mendonca changed the title Highlight leading and trailing spaces Highlight leading and trailing spaces, and double/multiple spaces in the middle Oct 20, 2022
@pedro-mendonca
Copy link
Member Author

Added unit tests for both the spaces highlight introduced in this PR, and the already existent markup for invisible characters as tabs and line breaks

@akirk
Copy link
Member

akirk commented Mar 8, 2023

We agreed on our triage session that this can be merged after a code review. The functionality is fine.

@amieiro amieiro merged commit b754d68 into GlotPress:develop Apr 19, 2023
@pedro-mendonca pedro-mendonca deleted the invisible-spaces branch April 19, 2023 10:01
@lmwp
Copy link

lmwp commented Aug 9, 2023

just ... thank you so much !!!

@amieiro amieiro mentioned this pull request Oct 16, 2024
Open
@amieiro amieiro mentioned this pull request Oct 24, 2024
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amieiro amieiro amieiro approved these changes

@ocean90 ocean90 Awaiting requested review from ocean90

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Highlight trailing spaces
4 participants
@pedro-mendonca @akirk @lmwp @amieiro