Fix(types): add missing reference and popper class properties
#759
Conversation
| @@ -158,6 +158,8 @@ declare class Popper { | |||
| static Defaults: PopperOptions; | |||
|
|
|||
| options: PopperOptions; | |||
| popper: HTMLElement; | |||
| reference: HTMLElement; | |||
|
|
|||
| constructor(reference: Element | ReferenceObject, popper: Element, options?: PopperOptions); | |||
There was a problem hiding this comment.
reference: Element, not HTMLElement?
There was a problem hiding this comment.
it can also be "custom reference object", we should already have a type for that somewhere
There was a problem hiding this comment.
Shouldn't have done this at the end of the work day...
It should be:
popper: Element;
reference: Element | ReferenceObject;
I'll try to test and commit before leaving today.
There was a problem hiding this comment.
It is a bit conflicting also, because in the code you have HTMLElement types for popper and reference, and Element in type definitions.
I'm not sure which one should be the correct one.
See this stackoverflow answer for details.
There was a problem hiding this comment.
The JSDoc types are most likely wrong... But I don't have much time to verify it
There was a problem hiding this comment.
It can be an SVGElement too. So that's why it's the base Element interface
Add missing `ReferenceObject` possible type for `reference`
|
It seems good now! |
|
@medfreeman may you double check the Flow types are in sync please? |
|
sure, about this feature only or in general ? |
|
I'll have to do it tomorrow morning anyway. |
|
I was thinking just about this specific fix. But if you find other issues feel free to send PRs :-) |
|
I won't hesitate! |
Reference: https://github.com/FezVrasta/popper.js/blob/bd1faa9a27499433443bb4c42b3dea9ecf055777/packages/popper/src/index.js#L37-L38