fix: flyout id is different than first placed block#7618
fix: flyout id is different than first placed block#7618BeksOmega merged 7 commits intoRaspberryPiFoundation:developfrom
Conversation
BeksOmega
left a comment
There was a problem hiding this comment.
First look seems good =) I'll pull it down and give it a manual test in a bit!
Would you also be interested in writing a tiny unit test in our block serialization suite that tests that when saveIds is false, there aren't IDs?
Thank you for your work on this @treypisano :D
core/serialization/blocks.ts
Outdated
| block: Block, | ||
| state: State, | ||
| doFullSerialization: boolean, | ||
| addIds: boolean, |
There was a problem hiding this comment.
Can you make this saveIds so that it's consistent?
|
Sure, I'll write that test. |
BeksOmega
left a comment
There was a problem hiding this comment.
Just one fix and then LGTM!
core/serialization/blocks.ts
Outdated
| const state = { | ||
| 'type': block.type, | ||
| 'id': block.id, | ||
| 'id': saveIds ? block.id : null, |
There was a problem hiding this comment.
We actually want this to be undefined instead of null. null gets stringified and takes up space in the save data, but undefined does not get serialized.
E.g. if you pass an object like the following to JSON.stringify:
{
propertyA: null,
propertyB: undefined
}
You get:
'{"propertyA": null}'
There was a problem hiding this comment.
Got it, I hadn't thought of it that way
BeksOmega
left a comment
There was a problem hiding this comment.
LGTM! Thank you for your work on this @treypisano =)
The basics
The details
Resolves
Fixes #7432
Proposed Changes
Adds a parameter,
saveIds: boolean, to thesavefunction. I also made sure that the parameter is passed through the recursive calls.Reason for Changes
This ensures that when a block is dragged out, the ID of the newly placed block is different than the one in the flyout.
Test Coverage
Documentation
Additional Information