Move projectChanged into HOC wrapping MenuBar, to avoid unnecessary renders

[mzgoddard]

Resolves

Less time rendering on project change and unchange.

Proposed Changes

Move projectChanged out of MenuBar into a parent component and pass MenuBar a function to test what the projectChanged logic was doing.

Reason for Changes

MenuBar uses projectChanged in some event handling behaviour. So when projectChanged changes, MenuBar renders leading to its tree being reconsidered when the nothing has changed.

Another possible implementation would be use a shouldComponentUpdate lifecycle handle. I figured the approach in this PR would be more robust (create less bugs).

Benchmark Data

These values are the amount of time spent deserializing scratch blocks from an sb3 project (173918262). The change (%) doesn't represent this very well since its over deserializing a projects blocks, when the change specifically applies to the first block being deserialized.

device	develop (ms)	menu-bar (ms)	difference (ms)	change (%)
chrome	268	254	14	5.15%
firefox	444	392	51	11.54%
safari	531	502	29	5.50%
chromebook	881	790	91	10.37%

[thisandagain]

@mzgoddard Can you take a look at the failing menu bar tests?

[mzgoddard]

I think they were all linting issues. I'll see how the tests are tomorrow if its also failing units/integrations.

[paulkaplan]

@benjiwheeler could you review this since you are working in a very similar area of code, and make sure that this approach could work with your change to the confirmation that you are working on?

For context, this PR extracts the confirmation logic into an HOC (something I know you were considering) but for a different reason–adding the projectChanged prop to the menubar only to use it in an event callback means that it will trigger unneeded re-renders (any render it triggers is unneeded, since it is not used in the rendering).

Additionally, it would also have the side benefit of making the logic for when to trigger a confirm more testable, since it extracts it from the menubar, like you were looking for.

[benjiwheeler]

@paulkaplan Yes, I'll review. This is one of the approaches I've looked at for where to put sb-file-uploader -- the holdup has been my concern about having the file picker loaded for everyone, all the time, when it's only used on rare occasions.

[benjiwheeler]

Tested this, it does indeed seem to avoid unnecessary renders. It's not a huge difference in number of renders, because the boundary between projectChanged being true and false isn't crossed often.

It seems like this is a subset of the container-esque functions and state that should be pulled out of the menu-bar component, eventually. See this issue: #2596 . So maybe this is the first step towards splitting menu-bar into container and presentation components.

[benjiwheeler]

LGTM. See my comments above. This may not be the format we use long term -- we may want to move much more of the menu bar component into a container like this.