paint-boolean-ops: fix toolbar and Redux initialization timing - #9164
paint-boolean-ops: fix toolbar and Redux initialization timing#9164griffpatch wants to merge 4 commits into
Conversation
mxmou
left a comment
There was a problem hiding this comment.
Looks good; the only things I'm not sure about are a comment and a function name.
|
Thanks @mxmou, both points make sense. I've updated the comment to explain why the Reshape sub-toolbar is watched separately, and renamed the helper to |
|
I have added a small follow-up for #9180. Open/Close now waits until the current paint mode is available. The toolbar helper also refreshes its native classes if Scratch adds the button groups after the toolbar row, which stops the Boolean buttons stacking onto separate rows. |
|
I'm happy with the fixes for If the changes here entirely subsume the changes in #9180 (which it looks like they do?) then I think it should either be marked as blocking so that it is merged first, or closed in favour of this. |
|
Thanks @pufferfish101007, yes, #9164 currently includes the same Redux changes as #9180. I'd prefer to keep the shared API fix separate for review, so I've marked this PR as blocked by #9180 in the description. Once that is reviewed and merged, I'll update this branch so only the paint changes remain. |
pufferfish101007
left a comment
There was a problem hiding this comment.
approving with the expectation of #9180 being merged first
Blocked by #9180. Please merge #9180 first. This PR currently includes the same Redux changes so the paint fix can use them. Once #9180 is merged, I will update this branch so only the paint changes remain.
The Open/Close button belongs to Scratch Paint's Reshape toolbar, but it was being updated along with the separate fixed toolbar and More menu.
On a narrow editor, this meant the button did not appear until More was opened. It could also stay behind after switching to Select or Text.
The button now follows the Reshape toolbar directly. It is removed when Reshape is no longer active and added again if React replaces that toolbar.
While checking this, I found that Open/Close was disabled for paths selected inside a Group or CompoundPath, even though Scratch can open and close those paths. The selection check now includes them. Releasing a compound path also keeps its children in the same parent.
Disabled Boolean operations in More now use the same disabled styling as Scratch's own items.
Follow-up after review
After the original review, I checked this alongside the Redux initialization issue in #7933.
Open/Close needs the current paint mode when the addon starts, so it now waits for Redux before checking that mode. This uses the Redux initialization change in #9180.
Waiting for Redux revealed that Scratch can create the fixed toolbar row before adding its button groups. The toolbar helper now refreshes the native classes when those groups appear, so the Boolean buttons stay on one row.
Fixes #9159