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

refactor: migrate scratch-blocks to Typescript #224

Merged
merged 125 commits into from
Nov 19, 2024
Merged

Conversation

gonfunko
Copy link
Owner

This PR migrates the scratch-blocks codebase to Typescript, and cleans up/modernizes a few files. Two files are unconverted:

  • checkable_continuous_flyout.js, because it calls layout_() on VerticalFlyout (bypassing its parent class), which is a protected method. Although it is a subclass, because its parent class (ContinuousFlyout) is JS, TS doesn't know this and complains. This is a temporary workaround that can be removed once the Continuous Toolbox plugin is updated to account for the new flyout API, at which point this file can be converted.
  • scratch_comment_bubble.js, because ISelectable requires a public workspace field. CommentView, the superclass of ScratchCommentBubble, has a workspace field, but it's private, so CommentView can't make it public to implement the interface. This will either need to be made public in core, or have the field in core named something else.

Copy link
Collaborator

@rachel-fenichel rachel-fenichel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but please ask CWF to look at the structure of it and sign off.

For later cleanup: many fields have a lot of underscores on private properties, which can now be renamed.

src/recyclable_block_flyout_inflater.ts Show resolved Hide resolved
src/fields/scratch_field_number.ts Show resolved Hide resolved
@gonfunko
Copy link
Owner Author

gonfunko commented Nov 1, 2024

@cwillisf let me know if this all looks good to you!

Copy link

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exciting!

src/blocks/data.ts Show resolved Hide resolved
src/scratch_variable_model.ts Show resolved Hide resolved
@gonfunko gonfunko merged commit 9a03eea into modern-blockly Nov 19, 2024
1 check passed
@gonfunko gonfunko deleted the ts-migration branch November 19, 2024 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants