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: refactoring translations to rely on readalong DOM instead of state #330

Merged
merged 5 commits into from
Sep 13, 2024

Conversation

deltork
Copy link
Collaborator

@deltork deltork commented Jul 29, 2024

PR Goal?

Rewrite of the translation tier to accomodate the creation of annotation layers

Fixes?

#328

Feedback sought?

sanity check

Priority?

high

Tests added?

How to test?

test with the following files ej-fra.readalong ,ej-fra-annotated.readalong,ej-fra-translated.readalong found in packages/web-component/test-data/ej-fra

Confidence?

high

Version change?

minor

TODO:

  • Translations

Copy link
Contributor

github-actions bot commented Jul 29, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-09-13 12:31 UTC

Copy link
Collaborator

@roedoejet roedoejet left a comment

Choose a reason for hiding this comment

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

Hey @deltork - I'm just reviewing this but I"m noticing that the "remove" 🗑️ button on the translation tiers doesn't work for removing tiers that exist in an uploaded readalong in the editor. Can you confirm?

@deltork
Copy link
Collaborator Author

deltork commented Aug 12, 2024

Hey @deltork - I'm just reviewing this but I"m noticing that the "remove" 🗑️ button on the translation tiers doesn't work for removing tiers that exist in an uploaded readalong in the editor. Can you confirm?

Fixed, pushed a work-around for version <1.1 with translation tiers without id

@deltork deltork force-pushed the dev.del/328-translation-tier-issue branch 2 times, most recently from 94f0875 to d52b764 Compare August 16, 2024 18:53
@deltork deltork marked this pull request as ready for review August 16, 2024 18:56
@deltork deltork force-pushed the dev.del/328-translation-tier-issue branch from d52b764 to 9225d2b Compare September 9, 2024 14:54
@deltork deltork requested review from joanise and roedoejet September 9, 2024 14:55
@deltork
Copy link
Collaborator Author

deltork commented Sep 9, 2024

@joanise I need translation help

@joanise joanise linked an issue Sep 11, 2024 that may be closed by this pull request
Copy link
Member

@joanise joanise left a comment

Choose a reason for hiding this comment

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

Question: OK with changing "annotation title" to "annotation layer" in the menu/widget to edit the layers?

Comment on lines 32 to 35
"edit-layer": "Rename annotation title",
"delete-layer": "Remove annotation title",
"save-layer": "Save annotation title",
"create-layer": "Create an annotation title"
Copy link
Member

Choose a reason for hiding this comment

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

These are not intuitive to me. I would change all of them to "annotation layer" instead of "annotation title".

Comment on lines 32 to 35
"edit-layer": "Renommer le titre de l'annotation",
"delete-layer": "Supprimer le titre de l'annotation",
"save-layer": "Enregistrer le titre de l'annotation",
"create-layer": "Créer un titre d'annotation"
Copy link
Member

Choose a reason for hiding this comment

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

And here "la/une couche d'annotation" instead of "le titre".
This is consistent with the translations I did for the tour.

Comment on lines 32 to 35
"edit-layer": "Cambiar el nombre del título de la anotación",
"delete-layer": "Eliminar el título de la anotación",
"save-layer": "Guardar el título de la anotación",
"create-layer": "Crear un título de anotación"
Copy link
Member

Choose a reason for hiding this comment

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

And "la/una capa de anotación".

Copy link
Member

@joanise joanise left a comment

Choose a reason for hiding this comment

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

This is now ready to merge as far as I'm concerned.

@deltork deltork merged commit 1fcfe58 into main Sep 13, 2024
2 checks passed
@deltork deltork deleted the dev.del/328-translation-tier-issue branch September 13, 2024 12:31
joanise added a commit that referenced this pull request Oct 16, 2024
*fix: refactoring translations to rely on readalong DOM instead of state (#330)

* fix: refactoring translations to rely on readalong DOM instead of state

* test: fix test for editing readalong

* feat: es and fr translations for layers messages

* fix: refine some translations and text strings

---------

Co-authored-by: Eric Joanis <[email protected]>
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.

When editing, the translation tier is not available for editing
3 participants