-
Notifications
You must be signed in to change notification settings - Fork 267
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
Pure JS implementation #457
base: master
Are you sure you want to change the base?
Conversation
Entity/TransUnitRepository.php
Outdated
@@ -210,6 +210,8 @@ protected function addTranslationFilter(QueryBuilder $builder, array $locales = | |||
|
|||
if ((is_countable($ids) ? count($ids) : 0) > 0) { | |||
$builder->andWhere($builder->expr()->in('tu.id', $ids)); | |||
} else { | |||
$builder->andWhere($builder->expr()->eq(1, 0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid to return all results (because there is no where condition added) when there is no ids matching the search. In case there is no matching results I add a condition to the builder that is never true.
With the fix when the search doesn't matches, we have an empty list:
We didn't wanted to throw a NoResult exception as we are in addTranslationFilter
and didn't wanted to change to much the logics but if you have a better idea, please suggest it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be in a separate PR if unrelated to the pure JS implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Seb33300 Right, I just revert 2 unrelated commit (this fix and the fix of deprecation)
|
||
spanColumnText.forEach(function (spanText) { | ||
params[spanText.column] = spanText.value; | ||
let tdElements = trElement.querySelectorAll('td[class^="col-"]'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this is shorter it doesn't read very easily. After a while you can make out that text is collected from td elements (which probably contain translations). Can you make it easier to follow somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
As proposed here #454, I have developed a pure JavaScript implementation while maintaining code isolation and similarities with the angularJs version to ease the review. The modifications are limited to translation.js and the grid-related Twig views.
The pure JS implementation support all configuration options (inputType, autoCacheClean, toggleSimilar) except the
profilerTokens
management, as I didn't fully grasp the concept.Few improvements have been done compared to the current AngularJs version pushed in separated commits:
Edit
Save