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

[Spritelab] Enable Start in Animation Tab level configuration #38711

Merged
merged 2 commits into from Jan 29, 2021

Conversation

ajpal
Copy link
Contributor

@ajpal ajpal commented Jan 23, 2021

First change: Don't dispatch the changeInterfaceMode event until after we've called StudioApp.init, which ensures that Blockly has been initialized properly. This fixes the following JS errors:
image

Second change: Rerender Blockly.mainBlockSpace when switching from the costume tab to the code tab. This render only seems necessary the first time you switch from costume to code, but we don't know whether it's necessary from within the change handler, so I have it just always re-rendering. I don't think that'll hurt performance because it's just one render, and Blockly rerenders things basically any time you drag a block anyways.

Before:
initial page load:
image
switch to code:
image
Then if you resize the window or the instructions/playspace panes:
image

After:
initial page load:
image
switch to code:
image

initial page load:
image
switch to code:
image

Links

Testing story

Reviewer Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@ajpal ajpal requested a review from a team January 23, 2021 01:12
@@ -359,6 +359,10 @@ P5Lab.prototype.init = function(config) {

this.studioApp_.init(config);

if (startInAnimationTab(getStore().getState())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm not sure i understand how moving this conditional from line 449 to line 362 fixed the issue we were seeing since this.studioApp_.init(config) is on line 360

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a little hard to follow because P5Lab.init() has a lot going on, but this.studioApp_.init(config) is inside another function (onMount), which is defined on line 324. So I'm moving the conditional to be after this.studioApp_.init(config) in that function, which is passed to P5LabView and called after that component is mounted

@ajpal ajpal merged commit bbacede into staging Jan 29, 2021
@ajpal ajpal deleted the jan22-start-in-animation-tab branch January 29, 2021 18:35
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.

None yet

2 participants