-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Open
Description
Context
We enabled strictMissingTypes and strictCheckProperties in the closure build as a prerequisite for running MigranTS. Where possible we fixed type annotations properly, but in other places we need to do a refactor to make the types make sense--particularly where we tack properties onto SVG Elements to pass information around.
Sub-issues
- Tooltip code:
.tooltipproperty on SVG ElementsmouseOverWrapperandmouseOutWrapperbeing stored on SVG Elements
- Touch/Gesture code uses
EventandPseudoEventinstead of more specific types-
PointerEventsare implemented on all of our supported browsers, so we can get rid of some of the touch code.
-
-
skewandtranslateon SVG Elements inblock_animations.js -
composeanddecomposeon block/blockSvg. Beka said: "Technically the compiler shouldn't accept this, because the BlockSvg versions of these functions expect more restricted types - which is bad. But also technically we always use the different versions correctly so it's fine. We just can't communicate that to the compiler without casting all the time." (See fix: add compose and decompose to block #6102) - Should
DragTargetbe an abstract class? -
setVisible_onIToolboxItem- Does it belong there? If yes, remove the underscore. Currently it's protected.
- Calls to
setFocusandselectingesture.jsare really only for workspace comments, but there's an issue for making focus and select work more uniformly: Workspace comments and all other bubbles should consistently handle single clicks. #1673 -
refreshThemein toolbox code.- Currently defined only on
ToolboxCategory. Toolbox'srefreshThemechecks if the function exists on all children, and calls it if it exists, because it's not in the interface.
- Currently defined only on
-
onKeyDown, used intoolbox.js.- It's not part of `ISelectableToolboxItem, and I don't think it's defined on any core toolbox items. Maybe it's used in one of the plugins. If yes, add to an interface. If no, delete it.
-
navigateBetweenStacks_inast_node.jsneeds type narrowing, but I can't tell what the possible types are.- @alschmiedt for this one.