-
Notifications
You must be signed in to change notification settings - Fork 61
docs(combineLatest): Add tip described in issue #143 #149
docs(combineLatest): Add tip described in issue #143 #149
Conversation
Codecov Report
@@ Coverage Diff @@
## master #149 +/- ##
==========================================
- Coverage 91.02% 88.46% -2.57%
==========================================
Files 7 7
Lines 78 78
Branches 5 7 +2
==========================================
- Hits 71 69 -2
Misses 6 6
- Partials 1 3 +2
Continue to review full report at Codecov.
|
| type: 'Tip', | ||
| text: ` | ||
| Note: combineLatest only start to emit when all sources have emitted at least once. | ||
| By adding a default start value to the sources with .startWith, it will activate right away. |
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.
Small typo, update only start with will only start . Also, since we are referencing startWith here can we make that a link? Thanks!
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.
For sure :) I'm onto 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.
Angular sanitizes the links inside the tips. Has this happened before anywhere?
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.
I've written a safeHtml pipe which returns this._sanitizer.bypassSecurityTrustHtml(html);, equivalent to the existing safeUrl pipe. Is this an acceptable solution for you?
d312028 to
0215b6a
Compare
|
Sorry for the commit message error, fixed it a minute ago. |
luillyfe
left a comment
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.
Looks good to merge 👍
284733d to
fc6e558
Compare
|
thx @nicolaisueper! can you rebase pls? |
fc6e558 to
d0bace2
Compare
|
@ladyleet done! |
|
Sorry @nicolaisueper, can you please rebase once more. |
|
@ashwin-sureshkumar There's only one commit inside this PR, I don't really know what I should do. Can you please explain to me how I should rebase? |
|
This is (almost) the first foreign open source project I'm contributing to, so please excuse my start difficulties :) |
|
Not a problem at all. Please follow below steps, I am being very explicit ignore if you have already done lot of these. The above should two remote addresses If you have only Then perform below steps Also, please refer https://github.com/ReactiveX/rxjs-docs/blob/master/CONTRIBUTING.md @nicolaisueper - Let me know if this help |
Point out that `combineLatest` starts to emit when all sources have emitted at least once and a hint to `startWith` for a default first value
d0bace2 to
6f6ae8a
Compare
|
@ashwin-sureshkumar Thanks a lot! I thought I should only rebase the commits inside this PR. Now rebased onto master. |
|
@nicolaisueper - Thank you ! |
|
This is so great! Thank you @nicolaisueper and @ashwin-sureshkumar! 💯 |
Point out that
combineLateststarts to emit when all sources have emitted at least once and a hintto
startWithfor a default first value