feat: add interface definitions which zone extends EventTarget#35304
feat: add interface definitions which zone extends EventTarget#35304JiaLiPassion wants to merge 1 commit intoangular:masterfrom
Conversation
6f8bdea to
8d99321
Compare
mhevery
left a comment
There was a problem hiding this comment.
Nits on docs, but love the result.
There was a problem hiding this comment.
/**
* ADD DOCS HERE.
*/
interface EventTarget {
As of right now the license is the comment of the EventTarget which is not what you meant. Please add EventTarget docs which are separate from the license header.
There was a problem hiding this comment.
You shouldn't add docs to an interface that is not owned by Angular imo. Not even sure that would even do anything at best or override the existing ones at worst.
A blank line to separate the licence should suffice, I reckon
There was a problem hiding this comment.
@mhevery , got it thanks. I updated the comment here.
@alfaproject , I checked the final result, both the comments from lib.dom and zone.js will display when user mouse hover EventTarget interface.
There was a problem hiding this comment.
Proper docs should have:
A single title sentence which goes to overview and should say just enough to tell the reader if this is what they want.
A paragraph describing the function in detail. Multiple sentences with space above it. Examples are always nice. This paragraph should be followed by `@param`
There was a problem hiding this comment.
Got it, I updated the comment, thanks.
4f6edd3 to
8c99b87
Compare
a563555 to
2e8d6a2
Compare
2e8d6a2 to
5a02204
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Close #35173
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #35173
What is the new behavior?
Does this PR introduce a breaking change?
add interfaces that zone.js extends EventTarget.