Translations for tab-alpha.js and vocab-search.js (excluding Aria tags) (#1722)#1723
Conversation
… in the search autocomplete have been changed from using the SKOSMOS object to the new method with Vue components
…ges were updated to fit with the agreed translation methodss used in Vue components
…e during a merge.
… in the search autocomplete have been changed from using the SKOSMOS object to the new method with Vue components
…ges were updated to fit with the agreed translation methodss used in Vue components
…ttps://github.com/NatLibFi/Skosmos into issue1722-add-translations-for-front-end-components
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1723 +/- ##
============================================
- Coverage 73.13% 70.90% -2.24%
+ Complexity 1958 1650 -308
============================================
Files 33 33
Lines 4769 4330 -439
============================================
- Hits 3488 3070 -418
+ Misses 1281 1260 -21 ☔ View full report in Codecov by Sentry. |
UnniKohonen
left a comment
There was a problem hiding this comment.
There are a few minor chnages I'd make:
- A computed value should be used for the loading message in alphabecal tab component
- The
$tfunction could be called directly in thetranslateTypefunction andtranslateTypescould be removed in vocab search component
A bigger issue is that vocab counts, alphabetical tab and vocab search Vue components all define and call the function waitForTranslationService which means that only one of the components can be mounted. Each of the components should get its own function for waiting for the translations (like in the term counts component).
|
This PR seems to contain commits merged from the main branch (e.g. the PHPUnit 10 upgrade), which make the commit history and GitHub diff view a bit messy. Maybe it would help to switch the base branch briefly to something else and back to |
|
|
Now the merge conflicts have been resolved and the unnecessary 20 second waits in Cypress tests have been removed. 🎉 |
Review guidelines
Visually observable changes in the browser are primarily related to parts generated by the following files:
Test the user interface:
Note:
Reasons for creating this PR
Improves frontend translation functionalityies.
Link to relevant issue(s), if any
#1722
Description of the changes in this PR
Added similar features as in the previous Vue-related translation implementations.
Known problems or uncertainties in this PR
What if we explore the ways to implement the translation functionality using JavaScript's promise features. By doing so, translations could be loaded in a lazy manner, meaning that if translations need to be awaited, the translation key would briefly appear to the user (likely some milliseconds), but as soon as the translations are ready, the key would be replaced with the actual translation.
This is a very common way to implement similar functionalities. The current implementation works but it has a weakness: if additional dependencies will be needed later that also need to be awaited before the page loads, they must be added to the same functionality (risk of excessive complexity).
Therefore, using the async -> await -> response -> .then -> .catch structure could be considered.
Checklist
.sr-onlyclass, color contrast)