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

store block comment position in block data #10164

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

riknoll
Copy link
Member

@riknoll riknoll commented Sep 9, 2024

fixes microsoft/pxt-microbit#5889

one of the additions we made in pxt-blockly was the ability for block comments to serialize their position to the XML when saving workspaces. unfortunately, we can't do the same thing with unforked blockly because there are no extension points in the XML serialization of comments. however, blockly does have a data element on blocks that you can stick arbitrary data into and will get saved in the XML (we already use this for persisting data for some of our field editors)

this pr brings back the saving of the comment position by shoving it into the block's data element. note that this will not fix existing projects from before the blockly upgrade, but i really don't think anyone will notice.

@riknoll riknoll requested a review from a team September 9, 2024 17:36
if (this.textInputBubble) {
const coord = this.textInputBubble.getPositionRelativeToAnchor();

setBlockDataForField(this.sourceBlock, COMMENT_OFFSET_X_FIELD_NAME, coord.x + "");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the + "" needed for the coordinate?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's a number, needs to be a string

Copy link
Contributor

Choose a reason for hiding this comment

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

Woah, I didn't know that was a thing, thanks for the info!

Copy link
Contributor

@srietkerk srietkerk left a comment

Choose a reason for hiding this comment

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

One small question, otherwise looks good.

@riknoll riknoll merged commit f2f9870 into master Sep 9, 2024
6 checks passed
@riknoll riknoll deleted the dev/riknoll/comment-position-serialize branch September 9, 2024 18:59
abchatra pushed a commit that referenced this pull request Sep 12, 2024
* store block comment position in block data

* prevent empty string comments from persising saved offset data
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.

Comments do not remember position when hidden and re-shown
2 participants