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

implement event listener for notifying when translations are ready #1772

Merged
merged 2 commits into from
Mar 13, 2025

Conversation

osma
Copy link
Member

@osma osma commented Mar 12, 2025

Reasons for creating this PR

Smarter event handling for frontend components that need to be initialized only after the translation service is ready.

See #1771

Link to relevant issue(s), if any

Description of the changes in this PR

  • add onTranslationReady function for registering event handlers that should be called when translations are ready
  • use onTranslationReady in Vue components instead of stupid polling boilerplate code

Known problems or uncertainties in this PR

n/a

Checklist

  • phpUnit tests pass locally with my changes
  • I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
  • The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
  • The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@osma osma added this to the 3.0 milestone Mar 12, 2025
@osma osma self-assigned this Mar 12, 2025
@osma
Copy link
Member Author

osma commented Mar 13, 2025

Hmm, I think there's a race condition that still needs to be fixed.

@osma osma marked this pull request as draft March 13, 2025 10:43
Copy link
Contributor

@UnniKohonen UnniKohonen left a comment

Choose a reason for hiding this comment

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

LGTM!

@osma osma marked this pull request as ready for review March 13, 2025 11:40
@osma
Copy link
Member Author

osma commented Mar 13, 2025

I added a check for the case when callbacks are registered after the translation service is initialized, and made sure those are called immediately. I tested it by making a 1000ms delayed call to onTranslationReady and it seemed to work. AFAICT, this special case can't happen with the current onTranslationReady calls we have, but it might happen later, so it's better to be able to handle that already.

All done, merging.

@osma osma merged commit 53d66fb into main Mar 13, 2025
10 of 11 checks passed
@osma osma deleted the issue1771-translation-service-callback branch March 13, 2025 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Issue/PR closed
Development

Successfully merging this pull request may close these issues.

Add callback notification facility to the JS translation service
2 participants