feat(notification): Allow templateRef as notification content #95
Conversation
The jest-preset-angular already has the necessary types. If both get installed, then we get build-time errors due to duplicate type definition files
Codecov Report
@@ Coverage Diff @@
## develop #95 +/- ##
===========================================
+ Coverage 99.13% 99.13% +<.01%
===========================================
Files 14 14
Lines 460 461 +1
Branches 79 79
===========================================
+ Hits 456 457 +1
Misses 4 4
Continue to review full report at Codecov.
|
itsdevdom
left a comment
itsdevdom
left a comment
There was a problem hiding this comment.
Implementation looks very good! Could you also add a test case for the new template functionality?
| <path d="M19 6.41L17.59 5 12 10.59 6.41 5 5 6.41 10.59 12 5 17.59 6.41 19 12 13.41 17.59 19 19 17.59 13.41 12z" /> | ||
| </svg> | ||
| </button> | ||
| <ng-container *ngIf="notification.template; else predefinedNotification" [ngTemplateOutlet]="notification.template" [ngTemplateOutletContext]="{ notification: notification }"> |
There was a problem hiding this comment.
I'm assuming a shorthand syntax like [ngTemplateOutletContext]="{ notification }" wouldn't work?
There was a problem hiding this comment.
Hmm... not sure actually.
|
@dominique-mueller
Sure..actually I already had that, but apparently forgot to push that commit as well. Gonna check that and push it up again 👍 |
|
@dominique-mueller In fact, the test was already there. For some reasons the file has just been reformatted (probably) which is why it seems like the entire file has been committed. Here's the test I added: |
|
Ah yes, you're right, that's probably why I've overlooked it. Any chance you could fix the formatting issue? Seems like the line breaks are not correct. |
Why bother, use Prettier? 😉 If you want, I can try to look into it again...probably taking your version and just pasting in my modifications...have to see |
|
Seems to be an issue with line breaks, in Visual Studio Code you can easily convert them :) |
juristr
force-pushed
the
feat/dynamic-templates
branch
from
fad9457 to
223733a
Compare
|
Just tried again...no luck apparently. What line endings are you using in your setting? P.S. I highly highly recommend going with prettier. avoids these kind of problems 😉. Please let me know. I'm travelling today and tomorrow to speak at ngtalks.io, but would really like to have this merged by Friday in order to conclude some work in my app :). thx |
itsdevdom
changed the title
|
Merged, and release as |
|
I thought about prettier as well, but it doesn't 100% match with my personal style guide, in particular with the wide spacing. And I'm not yet ready to give up yet ;D But for sure a formatter would be great! |
|
@dominique-mueller thx for merging 😄. But looks like there's an issue. Seems as if the src code rather than the compiled package has been published on NPM 😅 |
|
Ah damnit, you're right. I'll fix it with |
|
Just published |
2 participants
There are basically two things provided here in this PR
Fixes a compilation issue due to duplicate type definitions by
@types/jestand thejest-preset-angularextends the current implementation to allow passing in custom templates.
p.s.: the tests were a bit hard to write because I needed to get a
TemplateRef, which is easiest by creating a "Test component" on the fly, which was kind of cumbersome because theTestingModuleis created for each test case & also extracts services by using the TestBed injector (i.e.TestBed.get(...)). After you do that, you cannot override a component's template on the fly. Anyway, I added a flag for my test and it works.