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

fix: upgrade shareable procedures for new clipboard API #1863

Conversation

BeksOmega
Copy link
Contributor

@BeksOmega BeksOmega commented Aug 16, 2023

Closes #1838
Closes google/blockly#6846

Upgrades the shareable procedures plugin to use the new doFullSerialization parameter.

@BeksOmega BeksOmega force-pushed the fix/upgrade-shareable-for-clipboard branch from 9b339e4 to 2f56273 Compare August 16, 2023 22:21
@BeksOmega BeksOmega marked this pull request as ready for review August 16, 2023 22:41
@BeksOmega BeksOmega requested a review from a team as a code owner August 16, 2023 22:41
@BeksOmega BeksOmega requested review from rachel-fenichel and removed request for a team August 16, 2023 22:41
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.

Two questions about the intent of the code.

'paramId': p.getId(),
};
});
}
}
if (!this.hasStatements_) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you only need to set this when hasStatements_ is false, rather than always?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default is true. We always skip serialization of defaults (or we try to anyways) because it keeps the save file size smaller. E.g. we also don't serialize a 'params' array if the procedure has no params.

const procedureId = state['procedureId'];
if (procedureId && procedureId != this.model_.getId() &&
map.has(procedureId) &&
(this.isInsertionMarker() ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you no longer have to check whether it's an insertion marker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Insertion markers are saved with doFullSerialization: false so it is covered by the other check.

@BeksOmega BeksOmega merged commit 22c09e2 into google:2023-q3-eq-release Aug 16, 2023
9 checks passed
BeksOmega added a commit to BeksOmega/blockly-samples that referenced this pull request Sep 7, 2023
* chore: fix tests

* fix: upgrade the shareable procedure blocks to use the new clipboard system

* chore: update blockly version

* chore: fix lint
BeksOmega added a commit to BeksOmega/blockly-samples that referenced this pull request Sep 19, 2023
* chore: fix tests

* fix: upgrade the shareable procedure blocks to use the new clipboard system

* chore: update blockly version

* chore: fix lint
BeksOmega added a commit to BeksOmega/blockly-samples that referenced this pull request Sep 19, 2023
* chore: fix tests

* fix: upgrade the shareable procedure blocks to use the new clipboard system

* chore: update blockly version

* chore: fix lint
BeksOmega added a commit to BeksOmega/blockly-samples that referenced this pull request Sep 28, 2023
* chore: fix tests

* fix: upgrade the shareable procedure blocks to use the new clipboard system

* chore: update blockly version

* chore: fix lint
BeksOmega added a commit to BeksOmega/blockly-samples that referenced this pull request Sep 28, 2023
* chore: fix tests

* fix: upgrade the shareable procedure blocks to use the new clipboard system

* chore: update blockly version

* chore: fix lint
BeksOmega added a commit that referenced this pull request Sep 28, 2023
* chore: fix tests

* fix: upgrade the shareable procedure blocks to use the new clipboard system

* chore: update blockly version

* chore: fix lint
BeksOmega added a commit that referenced this pull request Sep 28, 2023
* fix: upgrade keyboard navigation to use new clipboard API (#1844)

* fix: upgrade keyboard navigation to use new clipboard API

* chore: reorganize test suites

* chore: fixup delete tests

* chore: work on fixing copy tests

* chore: finish fixing up copy tests

* chore: fix cut and paste tests

* chore: delete paste suite

* chore: delete unnecessary test helpers

* chore: fixup other tests

* chore: document why we're stubbing cancelAnimationFrame and add it to teardown

* fix: upgrade cross tab copy paste to use new clipboard API (#1845)

* fix: upgrade cross tab copy paste to use new clipboard API

* chore: update to beta

* fix: upgrade shareable procedures for new clipboard API (#1863)

* chore: fix tests

* fix: upgrade the shareable procedure blocks to use the new clipboard system

* chore: update blockly version

* chore: fix lint

* fix: serialization of dynamic if blocks (#1880)

* fix: normalize inputs

* fix: work on fixing serialization

* chore: remove inputCounts

* chore: fix tests

* fix: infinite loop

* fix: serialization of dynamic list blocks (#1885)

* fix: normalize inputs

* fix: serialization

* chore: remove inputCounter

* fix: normalize connections from load

* chore: switch tests to regexp

* chore: refactor tests

* fix: rebuilding behavior

* chore: revert changes to playground

* chore: format

* fix: fully tear down dynamic block before rebuilding (#1886)

* fix: dynamic text serialization (#1887)

* fix: normalize inputs

* fix: serialization

* chore: remove inputCounter

* fix: normalize connections from load

* chore: switch tests to regexp

* chore: refactor tests

* feat: dynamic json serialization (#1888)

* feat: add json serialization for text and list dynamic blocks

* chore: use helpers to append inputs

* chore: add json serialization to if blocks

* fix: support loading stringified XML from JSOn

* fix: dynamic connection plugin readme (#1919)

* chore: update readme and remove warning

* chore: add backticks

* deprecate: edit row separator readme to deprecate it (#1920)

* chore: edit row separator readme to deprecate it.

* Mark deprecated package private to stop publishing

* fix: unnamed callers for shareable procedures (#1983)

* fix: rendering dynamic connections (#1984)

* fix: migrating json for dynamic connections (#1985)

* chore: get tests to fail

* fix: round tripping old json to new json

* chore: fixup fixes

* chore: give up on getting tests to fail properly

* fix: fixup keyboard nav to remove blockly version check (#1991)

* fix: fixup keyboard nav to remove check

* chore: update beta version

* fix: bump blockly to v10.2.0

---------

Co-authored-by: John Nesky <[email protected]>
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.

2 participants