Sprite selector, CSS and Redux overhaul #25

rschamp · 2016-12-07T22:34:29Z

Believe it or not this a WIP PR. But it's getting out of hand so I figured I should submit it for review now before I do anything more.

Towards #14.
Requires scratchfoundation/scratch-vm#353.

This is probably better suited for a walk-through so let me know if you want to do that instead of trying to figure out everything this does.

Add TargetSelector component, comprised of SpriteSelector and StageSelector. Eventually it will also contain the "sprite info pane" in some form.
Refactor the various media library modals/buttons to be the responsibility of the TargetSelector
Display costumes/backdrops for each sprite and the stage in the SpriteSelector and StageSelector, respectively.
Use redux to centralize the VM state for use in various components. In this case, the StageSelector and SpriteSelector should share the VM's target list state. This state requires listening to both SPRITE_INFO_REPORTs and targetsUpdate events from the VM and reducing them into a single object. After I wrote the logic for this in the component I realized I was just poorly re-inventing redux, so I switched to using redux for this. After doing that, I realized it could also take the responsibility of the "VM manager" object.
Apply redux to the modals, just because it simplified the structure of the TargetSelector component.
Stop using inline styles, use CSS modules instead. I gave inline styles a shot, but it just seemed like it wasn't going to scale well, and it's not very user-friendly to have CSS written in JS. The local scoping that CSS modules provide give the same benefits of inline styles without sacrificing ergonomics.
Do a pass on the layout of the GUI: there just wasn't room for the sprite selector with the green flag and stop buttons where they were, so I moved things around.
Give the GUIComponent responsibility over the overall layout. With the GUI container controlling the set of components, eventually I would need to be putting layout styles in the container, which isn't its responsibility. So move this to the GUI component instead.

In preparation to split the sprites from the stage, start managing the target list state in a single place.

Add a new stage selection component that displays the stage separately from the sprite list.

Refactor SpriteSelector into "Target Pane" which contains the sprite selector, stage selector, and the buttons for new sprites, costumes and backdrops.

The inline css method gives us nothing that CSS modules can't give us. This puts the project more in line with the way www works, and makes CSS actually readable. Using class names gives us full control of the modal styles.

This will allow the GUIComponent to have control over the layout

cwillisf

Looks good!

Overall impressions:

Even in their WIP state I think I like how the libraries are shaping up.
I find the reducers pretty hard to understand, but that might just be me & my limited Redux experience.
How would you feel about going further in an event-driven (or Redux-style) direction? I'm looking at the big list of forwarded props in the TargetPane component and wishing that TargetPane didn't have to know about all of them...

src/components/blocks/blocks.jsx

        return (
            <div
-                className="scratch-blocks"
+                className={styles.blocks}


src/components/gui/gui.jsx

+            <TargetPane
+                mediaLibrary={mediaLibrary}
+                vm={vm}
+                {... targetPaneProps}


src/components/modal/modal.jsx

+                    className={styles.modalCloseButton}
+                    onClick={this.props.onRequestClose}
+                >
+                    {'x'}


rschamp · 2016-12-09T20:38:01Z

How would you feel about going further in an event-driven (or Redux-style) direction? I'm looking at the big list of forwarded props in the TargetPane component and wishing that TargetPane didn't have to know about all of them...

Yeah this was something that was annoying me too. It would be nice if the containers/components could remain self-contained and only list the PropTypes for the props that they explicitly use. But because of the pattern where a container passes props to the component, the containers effectively require the props of their components.

If I blindly pass the ... rest props to the component from the container, and don't list them in propTypes then the linter won't complain. If you then failed to pass all the required props to the container, the component would throw the error. That's more correct than the container throwing the error since the component is the one missing props. On the other hand, it would be harder to figure out why that's happening since the error would come from a deeper level than the source of the problem... So I'm not sure which is better.

src/components/sprite-selector/sprite-selector.jsx

+        >
+            {Object.keys(sprites)
+                // Only render ready sprites
+                .filter(id => 'costume' in sprites[id] && 'order' in sprites[id])


src/components/target-pane/target-pane.jsx

+                backdropCount={stage.costumeCount}
+                id={stage.id}
+                selected={stage.id === editingTarget}
+                url={stage.costume && stage.costume.skin}


src/components/target-pane/target-pane.jsx

+                onSelect={onSelectSprite}
+            />
+            <p className={styles.targetPaneLibraryButtons}>
+                <button onClick={onNewSpriteClick}>New Sprite</button>


fixed layout bug

# [4.1.0-hotfix.1](v4.0.38...v4.1.0-hotfix.1) (2024-10-19) ### Bug Fixes * add pinch to zoom support ([#21](#21)) ([a690749](a690749)) * add support for Scratch-style procedures ([#6](#6)) ([df207e4](df207e4)) * adjust key event filtering to fix the when key pressed block ([#13](#13)) ([06b011f](06b011f)) * allow typing into comments ([#15](#15)) ([668fbb9](668fbb9)) * avoid clearing the state of the sensing_of block by refreshing the toolbox ([#28](#28)) ([9bd036b](9bd036b)) * call reportValue on the ScratchBlocks module instead of the workspace ([#11](#11)) ([1898c52](1898c52)) * call the new glow methods on ScratchBlocks ([#12](#12)) ([be4386d](be4386d)) * fix bug that prevented displaying the procedure editor modal on mobile ([#26](#26)) ([8b1bf24](8b1bf24)) * make dropdown menu shadow block colors consistent ([#22](#22)) ([45d4803](45d4803)) * modify inject options to reflect Scratch behaviors ([#5](#5)) ([59bf3c5](59bf3c5)) * only refresh the toolbox when procedures are created via undo ([#19](#19)) ([f6dfd53](f6dfd53)) * partially roll back flyout optimization ([#25](#25)) ([1745c59](1745c59)) * patch the getCheckboxState method in the new flyout ([#7](#7)) ([dd98dd8](dd98dd8)) * preserve toolbox scroll position when switching between sprites/the stage ([#10](#10)) ([c8bc880](c8bc880)) * prevent exception when switching languages ([#27](#27)) ([d174ae9](d174ae9)) * reenable the Scratch colour eyedropper ([a7b1a10](a7b1a10)) * select extension categories when added ([#8](#8)) ([2d58403](2d58403)) * show the correct toolbox based on sprites or the stage being selected ([#4](#4)) ([dbbb251](dbbb251)) * specify the function to be used for prompting about variables ([#17](#17)) ([ee80f13](ee80f13)) * temporarily disable functionality in scratch-gui for compatibility with patched scratch-blocks ([#1](#1)) ([905f043](905f043)) * update the toolbox in response to procedure deletion/creation ([#18](#18)) ([505011d](505011d)) * Use toolboxitemid instead of id as the identifier attribute for toolbox categories ([#9](#9)) ([077415b](077415b)) ### Features * plumb Scratch variable support into the UI ([#16](#16)) ([c92f244](c92f244))

# [4.1.0-beta.1](v4.0.38...v4.1.0-beta.1) (2024-10-19) ### Bug Fixes * add pinch to zoom support ([#21](#21)) ([a690749](a690749)) * add support for Scratch-style procedures ([#6](#6)) ([df207e4](df207e4)) * adjust key event filtering to fix the when key pressed block ([#13](#13)) ([06b011f](06b011f)) * allow typing into comments ([#15](#15)) ([668fbb9](668fbb9)) * avoid clearing the state of the sensing_of block by refreshing the toolbox ([#28](#28)) ([9bd036b](9bd036b)) * call reportValue on the ScratchBlocks module instead of the workspace ([#11](#11)) ([1898c52](1898c52)) * call the new glow methods on ScratchBlocks ([#12](#12)) ([be4386d](be4386d)) * fix bug that prevented displaying the procedure editor modal on mobile ([#26](#26)) ([8b1bf24](8b1bf24)) * make dropdown menu shadow block colors consistent ([#22](#22)) ([45d4803](45d4803)) * modify inject options to reflect Scratch behaviors ([#5](#5)) ([59bf3c5](59bf3c5)) * only refresh the toolbox when procedures are created via undo ([#19](#19)) ([f6dfd53](f6dfd53)) * partially roll back flyout optimization ([#25](#25)) ([1745c59](1745c59)) * patch the getCheckboxState method in the new flyout ([#7](#7)) ([dd98dd8](dd98dd8)) * preserve toolbox scroll position when switching between sprites/the stage ([#10](#10)) ([c8bc880](c8bc880)) * prevent exception when switching languages ([#27](#27)) ([d174ae9](d174ae9)) * reenable the Scratch colour eyedropper ([a7b1a10](a7b1a10)) * **release:** release beta branch under beta label(?) ([e5cc322](e5cc322)) * select extension categories when added ([#8](#8)) ([2d58403](2d58403)) * show the correct toolbox based on sprites or the stage being selected ([#4](#4)) ([dbbb251](dbbb251)) * specify the function to be used for prompting about variables ([#17](#17)) ([ee80f13](ee80f13)) * temporarily disable functionality in scratch-gui for compatibility with patched scratch-blocks ([#1](#1)) ([905f043](905f043)) * update the toolbox in response to procedure deletion/creation ([#18](#18)) ([505011d](505011d)) * Use toolboxitemid instead of id as the identifier attribute for toolbox categories ([#9](#9)) ([077415b](077415b)) ### Features * plumb Scratch variable support into the UI ([#16](#16)) ([c92f244](c92f244))

# [4.1.0-spork.1](v4.0.39...v4.1.0-spork.1) (2024-10-21) ### Bug Fixes * add pinch to zoom support ([#21](#21)) ([a690749](a690749)) * add support for Scratch-style procedures ([#6](#6)) ([df207e4](df207e4)) * adjust key event filtering to fix the when key pressed block ([#13](#13)) ([06b011f](06b011f)) * allow typing into comments ([#15](#15)) ([668fbb9](668fbb9)) * avoid clearing the state of the sensing_of block by refreshing the toolbox ([#28](#28)) ([9bd036b](9bd036b)) * call reportValue on the ScratchBlocks module instead of the workspace ([#11](#11)) ([1898c52](1898c52)) * call the new glow methods on ScratchBlocks ([#12](#12)) ([be4386d](be4386d)) * fix bug that prevented displaying the procedure editor modal on mobile ([#26](#26)) ([8b1bf24](8b1bf24)) * make dropdown menu shadow block colors consistent ([#22](#22)) ([45d4803](45d4803)) * modify inject options to reflect Scratch behaviors ([#5](#5)) ([59bf3c5](59bf3c5)) * only refresh the toolbox when procedures are created via undo ([#19](#19)) ([f6dfd53](f6dfd53)) * partially roll back flyout optimization ([#25](#25)) ([1745c59](1745c59)) * patch the getCheckboxState method in the new flyout ([#7](#7)) ([dd98dd8](dd98dd8)) * preserve toolbox scroll position when switching between sprites/the stage ([#10](#10)) ([c8bc880](c8bc880)) * prevent exception when switching languages ([#27](#27)) ([d174ae9](d174ae9)) * reenable the Scratch colour eyedropper ([a7b1a10](a7b1a10)) * **release:** release beta branch under beta label(?) ([17c5b19](17c5b19)) * select extension categories when added ([#8](#8)) ([2d58403](2d58403)) * show the correct toolbox based on sprites or the stage being selected ([#4](#4)) ([dbbb251](dbbb251)) * specify the function to be used for prompting about variables ([#17](#17)) ([ee80f13](ee80f13)) * temporarily disable functionality in scratch-gui for compatibility with patched scratch-blocks ([#1](#1)) ([905f043](905f043)) * update the toolbox in response to procedure deletion/creation ([#18](#18)) ([505011d](505011d)) * Use toolboxitemid instead of id as the identifier attribute for toolbox categories ([#9](#9)) ([077415b](077415b)) ### Features * plumb Scratch variable support into the UI ([#16](#16)) ([c92f244](c92f244))

# [5.2.0-hotfix.1](v5.1.36...v5.2.0-hotfix.1) (2025-02-13) ### Bug Fixes * add pinch to zoom support ([#21](#21)) ([a690749](a690749)) * add support for Scratch-style procedures ([#6](#6)) ([df207e4](df207e4)) * allow typing into comments ([#15](#15)) ([668fbb9](668fbb9)) * avoid clearing the state of the sensing_of block by refreshing the toolbox ([#28](#28)) ([9bd036b](9bd036b)) * call reportValue on the ScratchBlocks module instead of the workspace ([#11](#11)) ([1898c52](1898c52)) * call the new glow methods on ScratchBlocks ([#12](#12)) ([be4386d](be4386d)) * fix bug that prevented displaying the procedure editor modal on mobile ([#26](#26)) ([8b1bf24](8b1bf24)) * fix preserving toolbox scroll position for new continuous toolbox plugin ([#30](#30)) ([b8f19dc](b8f19dc)) * make dropdown menu shadow block colors consistent ([#22](#22)) ([45d4803](45d4803)) * modify inject options to reflect Scratch behaviors ([#5](#5)) ([59bf3c5](59bf3c5)) * only refresh the toolbox when procedures are created via undo ([#19](#19)) ([f6dfd53](f6dfd53)) * partially roll back flyout optimization ([#25](#25)) ([1745c59](1745c59)) * patch the getCheckboxState method in the new flyout ([#7](#7)) ([dd98dd8](dd98dd8)) * preserve toolbox scroll position when switching between sprites/the stage ([#10](#10)) ([c8bc880](c8bc880)) * prevent exception when switching languages ([#27](#27)) ([d174ae9](d174ae9)) * reenable the Scratch colour eyedropper ([a7b1a10](a7b1a10)) * **release:** release beta branch under beta label(?) ([17c5b19](17c5b19)) * select extension categories when added ([#8](#8)) ([2d58403](2d58403)) * show the correct toolbox based on sprites or the stage being selected ([#4](#4)) ([dbbb251](dbbb251)) * specify the function to be used for prompting about variables ([#17](#17)) ([ee80f13](ee80f13)) * temporarily disable functionality in scratch-gui for compatibility with patched scratch-blocks ([#1](#1)) ([905f043](905f043)) * update "colour" property names for dark theme and mocks ([de06a2f](de06a2f)) * update the toolbox in response to procedure deletion/creation ([#18](#18)) ([505011d](505011d)) * Use toolboxitemid instead of id as the identifier attribute for toolbox categories ([#9](#9)) ([077415b](077415b)) ### Features * plumb Scratch variable support into the UI ([#16](#16)) ([c92f244](c92f244))

# [5.2.0-spork.1](v5.1.103...v5.2.0-spork.1) (2025-08-27) ### Bug Fixes * add pinch to zoom support ([#21](#21)) ([a690749](a690749)) * add support for Scratch-style procedures ([#6](#6)) ([df207e4](df207e4)) * adjust key event filtering to fix the when key pressed block ([#13](#13)) ([06b011f](06b011f)) * allow typing into comments ([#15](#15)) ([668fbb9](668fbb9)) * avoid clearing the state of the sensing_of block by refreshing the toolbox ([#28](#28)) ([9bd036b](9bd036b)) * call reportValue on the ScratchBlocks module instead of the workspace ([#11](#11)) ([1898c52](1898c52)) * call the new glow methods on ScratchBlocks ([#12](#12)) ([be4386d](be4386d)) * fix bug that prevented displaying the procedure editor modal on mobile ([#26](#26)) ([8b1bf24](8b1bf24)) * fix issue that could prevent variables from appearing in the toolbox ([#31](#31)) ([0570261](0570261)) * fix preserving toolbox scroll position for new continuous toolbox plugin ([#30](#30)) ([b8f19dc](b8f19dc)) * make dropdown menu shadow block colors consistent ([#22](#22)) ([45d4803](45d4803)) * modify inject options to reflect Scratch behaviors ([#5](#5)) ([59bf3c5](59bf3c5)) * only refresh the toolbox when procedures are created via undo ([#19](#19)) ([f6dfd53](f6dfd53)) * partially roll back flyout optimization ([#25](#25)) ([1745c59](1745c59)) * patch the getCheckboxState method in the new flyout ([#7](#7)) ([dd98dd8](dd98dd8)) * plumb `FieldNote` through to the VM ([#33](#33)) ([fb762c2](fb762c2)) * preserve toolbox scroll position when switching between sprites/the stage ([#10](#10)) ([c8bc880](c8bc880)) * prevent exception when switching languages ([#27](#27)) ([d174ae9](d174ae9)) * reenable the Scratch colour eyedropper ([a7b1a10](a7b1a10)) * **release:** release beta branch under beta label(?) ([17c5b19](17c5b19)) * select extension categories when added ([#8](#8)) ([2d58403](2d58403)) * show the correct toolbox based on sprites or the stage being selected ([#4](#4)) ([dbbb251](dbbb251)) * specify the function to be used for prompting about variables ([#17](#17)) ([ee80f13](ee80f13)) * temporarily disable functionality in scratch-gui for compatibility with patched scratch-blocks ([#1](#1)) ([905f043](905f043)) * update the toolbox in response to procedure deletion/creation ([#18](#18)) ([505011d](505011d)) * use serialization wrapper ([#32](#32)) ([6e46a18](6e46a18)) * Use toolboxitemid instead of id as the identifier attribute for toolbox categories ([#9](#9)) ([077415b](077415b)) ### Features * plumb Scratch variable support into the UI ([#16](#16)) ([c92f244](c92f244))

Ray Schamp added 10 commits December 2, 2016 15:22

Refactor libraries to be a part of SpriteSelector

e3b0069

Add custom sprite selector with costume display

865a291

Don't use node rules in src/ eslint config

ccc8a1e

Reduxify sprite selector

24389c3

In preparation to split the sprites from the stage, start managing the target list state in a single place.

Fix editingTarget selection

3cea22a

Reduxify modals

cf85a3b

Split stage selection from sprites

b29670b

Add a new stage selection component that displays the stage separately from the sprite list.

Style pass

d139609

Refactor SpriteSelector into "Target Pane" which contains the sprite selector, stage selector, and the buttons for new sprites, costumes and backdrops.

Use CSS modules for styles

16fbb64

The inline css method gives us nothing that CSS modules can't give us. This puts the project more in line with the way www works, and makes CSS actually readable. Using class names gives us full control of the modal styles.

Give GUIComponent responsibility of children

7542f1b

This will allow the GUIComponent to have control over the layout

rschamp added the pr - needs review label Dec 7, 2016

rschamp added this to the December 12 milestone Dec 7, 2016

rschamp self-assigned this Dec 7, 2016

rschamp requested a review from cwillisf December 7, 2016 22:34

rschamp mentioned this pull request Dec 7, 2016

Report costume data in sprite info reports and targetsUpdate scratchfoundation/scratch-vm#353

Merged

rschamp requested a review from tmickel December 7, 2016 23:09

cwillisf approved these changes Dec 9, 2016

View reviewed changes

tmickel reviewed Dec 11, 2016

View reviewed changes

src/components/sprite-selector/sprite-selector.jsx Outdated

>

{Object.keys(sprites)

// Only render ready sprites

.filter(id => 'costume' in sprites[id] && 'order' in sprites[id])

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

tmickel reviewed Dec 11, 2016

View reviewed changes

src/components/target-pane/target-pane.jsx

onSelect={onSelectSprite}

/>

<p className={styles.targetPaneLibraryButtons}>

<button onClick={onNewSpriteClick}>New Sprite</button>

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

tmickel approved these changes Dec 11, 2016

View reviewed changes

Ray Schamp added 2 commits December 12, 2016 11:18

Make ...rest syntax consistent.

5256237

Sprites always have costume data now

bd99101

rschamp added pr - good to merge and removed pr - needs review labels Dec 12, 2016

rschamp merged commit 71a8a30 into scratchfoundation:master Dec 12, 2016

rschamp deleted the feature/sprite-selector branch December 12, 2016 16:27

alexcui03 pushed a commit to Clipteam/scratch-gui that referenced this pull request Oct 1, 2020

Merge pull request scratchfoundation#25 from Clipteam/gui-layout

4271806

fixed layout bug

cwillisf pushed a commit that referenced this pull request Oct 11, 2024

fix: partially roll back flyout optimization (#25)

1745c59

sebjacobs pushed a commit to RaspberryPiFoundation/scratch-gui that referenced this pull request Apr 24, 2025

fix: partially roll back flyout optimization (scratchfoundation#25)

800641d

sebjacobs pushed a commit to RaspberryPiFoundation/scratch-gui that referenced this pull request Apr 30, 2025

fix: partially roll back flyout optimization (scratchfoundation#25)

792e569

sebjacobs pushed a commit to RaspberryPiFoundation/scratch-gui that referenced this pull request Apr 30, 2025

fix: partially roll back flyout optimization (scratchfoundation#25)

9983126

sebjacobs pushed a commit to RaspberryPiFoundation/scratch-gui that referenced this pull request Apr 30, 2025

fix: partially roll back flyout optimization (scratchfoundation#25)

e89a6b3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Sprite selector, CSS and Redux overhaul #25

Sprite selector, CSS and Redux overhaul #25

Uh oh!

rschamp commented Dec 7, 2016

Uh oh!

cwillisf left a comment

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

rschamp commented Dec 9, 2016 •

edited

Loading

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

Uh oh!

Sprite selector, CSS and Redux overhaul #25

Sprite selector, CSS and Redux overhaul #25

Uh oh!

Conversation

rschamp commented Dec 7, 2016

Uh oh!

cwillisf left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

rschamp commented Dec 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

Uh oh!

rschamp commented Dec 9, 2016 •

edited

Loading