fix!: fix or silence type errors #6105
Conversation
| * otherwise. | ||
| * @package | ||
| */ | ||
| IFlyout.prototype.isBlockCreatable; |
There was a problem hiding this comment.
Is removing the underscore on this name a breaking change?
I still don't understand our policy on external developers overriding @package properties in subclasses. But it seems like if this is part of the interface of a flyout, then renaming part of that interface is a breaking chnage.
There was a problem hiding this comment.
Technically a breaking change, so I renamed the PR to fix!.
There was a problem hiding this comment.
And also added a comment on the breaking change to the PR description.
| * @param {boolean} isVisible True if category should be visible. | ||
| * @protected | ||
| */ | ||
| IToolboxItem.prototype.setVisible_; |
There was a problem hiding this comment.
Is this supposed to exist here? Or on the collapsible toolbox item interface?
There was a problem hiding this comment.
It has to move all the way up to IToolboxItem because of how it's being used. @alschmiedt said that "we allow any toolbox item to be in a collapsible category" and that it should be left as is for compatibility reasons. But I noted it in my issue for follow-ups, in case we can find a better way to deal with it before the next release.
| @@ -743,8 +745,10 @@ class Gesture { | |||
| */ | |||
| doBubbleClick_() { | |||
| // TODO (#1673): Consistent handling of single clicks. | |||
| this.startBubble_.setFocus && this.startBubble_.setFocus(); | |||
| this.startBubble_.select && this.startBubble_.select(); | |||
| if (this.startBubble_ instanceof WorkspaceCommentSvg) { | |||
There was a problem hiding this comment.
Why are workspace comments bubbles? That seems like not what they are.
There was a problem hiding this comment.
Notes for the curious:
I think it's just to make workspace comments similar to block comments. And block comments are icons/bubbles, just like mutators.
Bubbles sit on a different canvas than blocks, and always on top. They do that so that blocks on the workspace can never look like they're inside an open mutator.
rachel-fenichel
changed the title
The basics
npm run formatandnpm run lintThe details
Resolves
Progress on #5857
Proposed Changes
Fix or silence remaining type errors exposed by enabling 'strictCheckTypes' and 'strictMissingProperties' flags for core code.
All changes resolve type errors. Where possible I made the types correct; when I couldn't, I added an item to track additional work in #6097. Most commits fix one error or type of error, so please review one commit at a time.
Things that blocked real fixes:
Behavior Before Change
~45 errors in core with those flags enabled.
Behavior After Change
No errors in core with those flags enabled.
Reason for Changes
These type errors were blocking conversion to TypeScript because the MigranTS tool wouldn't run with these errors in place.
Test Coverage
Tested for build errors by enabling 'strictCheckTypes' and 'strictMissingProperties' in
build_tasks.js.Tested runtime errors with unit tests and some basic dragging around in the playground.
Breaking change
Renames
isBlockCreatable_(on FlyoutBase) toisBlockCreatable. It was and still ispackage.