[Issue 1163] Add additional tests #1169
Conversation
24b7b2b (but keeping tests), and adding additional tests. 1163 appears to have actually been fixed by 1167/1164, as evidenced by the newly added tests.
| @@ -72,28 +72,21 @@ export class TranslateDirective implements AfterViewChecked, OnDestroy { | |||
| if (forceUpdate) { | |||
| node.lastKey = null; | |||
| } | |||
| if(isDefined(node.lookupKey)) { | |||
| key = node.lookupKey; | |||
There was a problem hiding this comment.
I believe that the lookup key will prevent the translation of an updated interpolated bound template variable.
| // the current content is the translation, not the key, use the last real content as key | ||
| key = node.originalContent.trim(); | ||
| } else if (content !== node.currentValue) { | ||
| // we want to use the content as a key, not the translation value | ||
| key = trimmedContent; | ||
| // the content was changed from the user, we'll use it as a reference if needed | ||
| node.originalContent = content || node.originalContent; | ||
| node.originalContent = content; |
There was a problem hiding this comment.
Because of the if (trimmedContent.length) conditional, content is guaranteed to be truthy, so the OR statement will never be executed.
| NESTED.TEST.KEY | ||
| </div> | ||
| <div #interpolatedWithSpaceAndLineBreakNoKeyNoParams translate> | ||
| {{ myInterpolatedTranslationKey }} |
There was a problem hiding this comment.
Adding tests for interpolated bound template variables.
| @@ -51,7 +53,7 @@ describe('TranslateDirective', () => { | |||
| ], | |||
| declarations: [App] | |||
| }); | |||
| translate = TestBed.inject(TranslateService); | |||
| translate = TestBed.get(TranslateService); | |||
There was a problem hiding this comment.
I saw that this had been recently changed, but the tests work either way, so I'm unsure why it changed?
If it needs to be .inject(), I'll happily put it back.
| const fr2="Un autre test"; | ||
| const setKey = key => { | ||
| fixture.componentInstance.myInterpolatedTranslationKey = key; | ||
| fixture.componentInstance.changeDetectorRef.markForCheck(); // Needed when changeDetection is ChangeDetectionStrategy.OnPush |
There was a problem hiding this comment.
I'm not sure why the test harness is configured with changeDetection: ChangeDetectionStrategy.OnPush, as this isn't the default for Angular. It's fine, but it also means that angular won't detect an interpolated variable change on its own, and we have to tell the change detector when to check for changes.
dpmott
requested review from
biesbjerg,
CodeAndWeb,
SamVerschueren and
tiaguinho
|
We're having the same issue and so far I moved from directive to pipes to mitigate the issue, but it would be perfect to be able to use directive in all cases :) |
|
@ocombe can you take a look into this? |
[Issue 1163] Add additional tests
EDIT: This also fixes issue #1176