fix: move the dropdown div to a namespace instead of a class with only static properties #5979
Conversation
| @@ -163,7 +160,8 @@ DropDownDiv.positionToField_ = null; | |||
| * height:number | |||
| * }} | |||
| */ | |||
| DropDownDiv.BoundsInfo; | |||
| let BoundsInfo; | |||
| exports.BoundsInfo = BoundsInfo; | |||
There was a problem hiding this comment.
@cpcallen does this work as expected, or does the type annotation need to be on the export? I'm not quite sure how that all interacts.
There was a problem hiding this comment.
This is how other type defs work I think. So hopefully should be good? If not I can go through and change those as well.
There was a problem hiding this comment.
I believe this is fine. And it will soon be a single export let …, which is definitely fine as far as annotating it goes.
core/dropdowndiv.js
Outdated
| @@ -387,18 +372,17 @@ const showPositionedByRect = function( | |||
| */ | |||
| const show = function( | |||
| owner, rtl, primaryX, primaryY, secondaryX, secondaryY, opt_onHide) { | |||
| owner_ = owner; | |||
| onHide_ = opt_onHide || null; | |||
| owner = owner; | |||
There was a problem hiding this comment.
Maybe make this newOwner to avoid shadowing?
There was a problem hiding this comment.
Whoops! Thought I caught all of these. I'll do another check.
There was a problem hiding this comment.
Ok I think this was actually the last one!
The basics
The details
Resolves
Maybe finishing #5860
Proposed Changes
Changes the dropdown div to be a namespace instead of a class that only has static properties. This includes changing the module-local variables to conform to the styleguide.
Should be no behavior changes, because I am reexporting the module under its previous name.
Reason for Changes
Workin' toward typescript.
Test Coverage
All unit tests pass.
Documentation
N/A
Additional Information
N/A