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: Multiline field not correctly size in parent block when copy/paste or load. #6429

Closed
wants to merge 1 commit into from

Conversation

yamadayutaka
Copy link
Contributor

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

Proposed Changes

When loading a block with input blocks or next blocks, render the blocks last.

Behavior Before Change

Multiline field not correctly size in parent block when copy/paste or load.

Behavior After Change

Multiline field correctly size in parent block when copy/paste or load.

Reason for Changes

These issues are from v7.
So I modified the processing order to be similar for v6.

https://github.com/google/blockly/blob/6.20210701.0/core/xml.js#L564-L569

@rachel-fenichel
Copy link
Collaborator

@BeksOmega can you take a look at the serialization changes?

@rachel-fenichel
Copy link
Collaborator

Hi @yamadayutaka--sorry for the delay, but the person who knows the serialization code best is currently out of office. We haven't forgotten about your change.

@BeksOmega BeksOmega requested review from BeksOmega and removed request for rachel-fenichel September 25, 2022 22:38
@BeksOmega
Copy link
Collaborator

Hi @yamadayutaka ! Thank you so much for looking into this :D Sorry it took me so long to get back to you!

I was wondering, instead of refactoring the serialization order, could we try using getFastTextWidth in updateSize_ for the FieldMultilineInput? This is what the FieldTextInput uses (via Field), which is why it doesn't run into the same problem.

I haven't looked at the serialization-based solution in depth, but I would like to try fixing the field itself first if possible.

@yamadayutaka
Copy link
Contributor Author

Hi @BeksOmega,
Thank you for reviewing my PR.
I will try it.

@yamadayutaka
Copy link
Contributor Author

I've confirmed that what you suggested works as expected, so I created another PR.
#6461

@BeksOmega
Copy link
Collaborator

Closing in favor of #6461

@BeksOmega BeksOmega closed this Oct 3, 2022
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