Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional type and cast cleanups #6097

Open
3 of 11 tasks
rachel-fenichel opened this issue Apr 21, 2022 · 0 comments
Open
3 of 11 tasks

Additional type and cast cleanups #6097

rachel-fenichel opened this issue Apr 21, 2022 · 0 comments

Comments

@rachel-fenichel
Copy link
Collaborator

rachel-fenichel commented Apr 21, 2022

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:
    • .tooltip property on SVG Elements
    • mouseOverWrapper and mouseOutWrapper being stored on SVG Elements
  • Touch/Gesture code uses Event and PseudoEvent instead of more specific types
    • PointerEvents are implemented on all of our supported browsers, so we can get rid of some of the touch code.
  • skew and translate on SVG Elements in block_animations.js
  • compose and decompose on 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 DragTarget be an abstract class?
  • setVisible_ on IToolboxItem
    • Does it belong there? If yes, remove the underscore. Currently it's protected.
  • Calls to setFocus and select in gesture.js are 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
  • refreshTheme in toolbox code.
    • Currently defined only on ToolboxCategory. Toolbox's refreshTheme checks if the function exists on all children, and calls it if it exists, because it's not in the interface.
  • onKeyDown, used in toolbox.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_ in ast_node.js needs type narrowing, but I can't tell what the possible types are.

See also

#6073
#5857

@rachel-fenichel rachel-fenichel added issue: triage Issues awaiting triage by a Blockly team member type: cleanup component: TypeScript and removed issue: triage Issues awaiting triage by a Blockly team member labels Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant