Add TypeScript declarations to iron-overlay-behavior. #261
Conversation
iron-overlay-behavior.d.ts
Outdated
| /** | ||
| * The backdrop element. | ||
| */ | ||
| backdropElement(): any; |
There was a problem hiding this comment.
Shouldn't this be of type Element?
https://github.com/PolymerElements/iron-overlay-behavior/blob/master/iron-overlay-behavior.html#L157
There was a problem hiding this comment.
Done. Looks like we're sometimes using the @type annotation for getters, but it should be @return. Updated. Also updated the other cases I found.
iron-overlay-behavior.d.ts
Outdated
| /** | ||
| * Returns the node to give focus to. | ||
| */ | ||
| _focusNode(): any; |
There was a problem hiding this comment.
iron-overlay-behavior.d.ts
Outdated
| * If you know what is your content (specifically the first and last focusable children), | ||
| * you can override this method to return only `[firstFocusable, lastFocusable];` | ||
| */ | ||
| _focusableNodes(): any; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Done. Actually it should be !Array<!Node> or else it would be a nullable array of nullable nodes, since non-primitive types are nullable by default in Closure. Updated the other cases I found too.
iron-overlay-behavior.d.ts
Outdated
| interface IronOverlayBehavior { | ||
| } | ||
|
|
||
| const IronOverlayBehavior: object; |
There was a problem hiding this comment.
how is it expressed that IronOverlayBehavior is composed by other behaviors?
https://github.com/PolymerElements/iron-overlay-behavior/blob/master/iron-overlay-behavior.html#L796
There was a problem hiding this comment.
Huh, that's a good catch. We don't handle behavior arrays right now. Filed https://github.com/Polymer/gen-typescript-declarations/issues/80 and will update this PR when done.
There was a problem hiding this comment.
Updated to add the other behaviors. See interface above.
| * Will call notifyResize if overlay is opened. | ||
| * Can be overridden in order to avoid multiple observers on the same node. | ||
| */ | ||
| _onNodesChange(): any; |
There was a problem hiding this comment.
is _manager property being omitted on purpose because it's private?
https://github.com/PolymerElements/iron-overlay-behavior/blob/master/iron-overlay-behavior.html#L132
There was a problem hiding this comment.
Yes, we don't emit private things.
iron-scroll-manager.html
Outdated
| @@ -56,6 +56,7 @@ | |||
| /** | |||
| * The current element that defines the DOM boundaries of the | |||
| * scroll lock. This is always the most recently locking element. | |||
| * @return {!Node} | |||
There was a problem hiding this comment.
currentLockingElement can be undefined - this was wrongly annotated previously as it was saying that currentLockingElement can be null
I'll update the tedium script to do this for all the repos (PolymerLabs/tedium#63), although did you have something else in mind? I guess I could make the generator itself have a flag that errors if anything has changed? |
aomarks
force-pushed
the
gen-typescript-declarations
branch
from
091dc84 to
8a5d32f
Compare
| @@ -0,0 +1,12 @@ | |||
| /** | |||
There was a problem hiding this comment.
@valdrinkoshi Is IronFocusablesHelper supposed to be something users use, or is it fine that this file is empty? It doesn't have any annotations, so analyzer doesn't pick it up as an interesting feature. Same question for IronScrollManager.
There was a problem hiding this comment.
All managers/helpers are singletons available on window and expose public methods, so can be used by users: IronFocusablesHelper, IronOverlayManager, IronScrollManager
I'm sure users rely on the public methods of IronFocusablesHelper and IronOverlayManager as they're been around for a while.
There was a problem hiding this comment.
I've added some annotations to generate IronScrollManager, although it is missing a getter. IronOverlayManager is probably too difficult for Analyzer to understand at all right now. Filed #262 with details on both.
I'd like to merge this PR despite these missing types, because this element is a dependency of a lot of other ones. I'd like to go back and add some manual typings for the managers a bit later, if that's ok.
|
Sounds good, LGTM |
This PR adds TypeScript declarations generated by https://github.com/Polymer/gen-typescript-declarations/
These declarations can be re-generated by running
npm run update-types.Tracker: https://github.com/Polymer/gen-typescript-declarations/issues/79