Add paint-gradient-editor addon: gradient stop editor for vector cost…#8918
Add paint-gradient-editor addon: gradient stop editor for vector cost…#8918griffpatch wants to merge 42 commits into
Conversation
Samq64
added
new addon
…e axis line visibility - Poll via rAF to detect modal close (CLOSE_MODAL not always dispatched) - Add black outline stroke behind white axis line for better visibility - Use class*= with trailing _ for CSS module selectors
…n first interaction
…unAtComplete:false timing)
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
This is a really useful addon, and I've wanted something like this for a while. Particularly the ability to fade out shapes at the edges.
The main functionality works well but there are some bugs though. I've commented directly at the lines that cause these bugs.
Additionally I think the addon description needs to describe how to use the addon in more detail. (for example the way you delete is definitely not something you can know without instructions)
Consider if drag off line to delete is actually better than double click to delete (the way scratch does it on their node editor)
Also the userscript file is very long and it might benefit from being split up into smaller files if possible.
| // handleGroup — SVG <g> handle group to highlight while picker is open (optional) | ||
| // ringRadius — ring radius in SVG units (default 11 for p0/p1 handles) | ||
| // openExtraColorPicker(idx, …) is a thin wrapper around this for extra stops. | ||
| const openColorPicker = (initialCss, onCommit, clientX, clientY, handleGroup = null, ringRadius = 11) => { |
There was a problem hiding this comment.
We already have third party colour picker vendor at libraries/thirdparty/color-picker.js
Please don't. That one needs to be replaced anyway because it is for Vue 1 and the source code is no longer available.
|
Before implementing a color picker, consider whether |
|
No it wasn't - I needed transparency, and the stage colour picker too... I
did try it with the standard one first, but didn't like it.
…On Sun, 29 Mar 2026 at 19:26, WorldLanguages ***@***.***> wrote:
*WorldLanguages* left a comment (ScratchAddons/ScratchAddons#8918)
<#8918?email_source=notifications&email_token=ABTM3PTZLMZMEAJ367T6TOD4TFTEZA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMJVGA3TQOBZGMZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2LK4DSL5RW63LNMVXHIX3POBSW4X3DNRUWG2Y#issuecomment-4150788932>
Before implementing a color picker, consider whether <input type="color">
is good enough.
—
Reply to this email directly, view it on GitHub
<#8918?email_source=notifications&email_token=ABTM3PTZLMZMEAJ367T6TOD4TFTEZA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMJVGA3TQOBZGMZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2LK4DSL5RW63LNMVXHIX3POBSW4X3DNRUWG2Y#issuecomment-4150788932>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABTM3PVPNE26NEXMETYXYFL4TFTEZAVCNFSM6AAAAACXCNGWTGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DCNJQG44DQOJTGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
… light/dark mode CSS Move the stop colour picker into stop-color-picker.js as a standalone ES module with all dependencies injected via the constructor. No imports — the module is pure logic with no coupling to the outer userscript. Replace inline <style> injection with static CSS classes in style.css. Picker panel colours now use --editorDarkMode-* variables with Scratch light-mode fallbacks, giving automatic light/dark mode support that matches the native colour picker popover.
…ition When clicking the axis to add a new stop, the offset was stored as the raw projected fraction t, but syncOverlay renders handles using crampedFrac which shifts positions inward to prevent overlap. The result was that the new handle appeared slightly away from the click point. Fix: invert the display mapping with crampedToOffset (computing the slot index the new stop will occupy after sorting) so the stored offset renders back at exactly the clicked position. Matches the same pattern already used by the p0 and extra-stop drag handlers.
… mixed selection !grd?.stops?.length >= 2 was parsed as (!grd?.stops?.length) >= 2, which evaluates to false >= 2 (always false), so the guard never skipped items without a gradient. In a mixed selection grd is undefined, causing grd.stops to throw TypeError and silently delete extra stops. Fix: replace with !grd || grd.stops.length < 2.
…n disable pollPickerClose now calls destroy() instead of sync() so the rAF loop stops when the native colour picker popover closes. On addon disable, close the floating stop-colour picker before destroying the SVG overlay so no stale panel is left on screen. Move all .sa-extra-stop-picker DOM knowledge into StopColorPicker.close(); userscript.js calls picker.close() or overlay.close() with no knowledge of the internal CSS class.
Joeclinton1
left a comment
Joeclinton1
left a comment
There was a problem hiding this comment.
So, the new behaviour is that if I select a shape with a gradient and no gradient the stop points go away which I guess fixes the guard bug. Either way I can't test that anymore. The other bugs seem fixed. And the new style matches the light mode of scratch better and reacts to dark mode.
Some bugs:
- The stop points are added at the right place if you click between the two existing nodes, however if you click outside those two it will place a stop point inside the two existing stop points rather than outside. The desired behaviour would be either to alert the user that you can't place a node outside the two starting stop points, or to just let the user do that instead of relocating it back to inside.
2026-03-30.12-38-11.mp4
- There is a bug where the fill preview for the shape begins with the generic multi shape selected icon, and then when you click the fill it shows the right preview. It should show that preview from the start.
- Sometimes drawing a new shape after interacting with a gradient will result in that shape having an empty fill
2026-03-30.12-41-26.mp4
UX:
- Would it be possible to double click a shape to enter gradient stop edit mode?
- I would still like the addon description updated to tell the user better how to use the addon.
- Consider a tool tip that appears after some delay of hovering a stop point to tell the user how to delete it.
… stops - Clicking the axis before p0 on a radial gradient promotes that position as the new p0, pushing the old p0 colour into extra stops as the first middle stop, then opens the p0 colour picker via a synthetic click. - Double-clicking an extra stop handle deletes it (same path as drag-off, via shared removeExtraStop() helper); guarded by attachDragMoved so a click-select then click-drag sequence is not misread as a double-click.
Okay, I fixed this now - clicking 'before' the first stop in a radial fill creates a new duplicate stop (which is hopefully less confusing - although you then can't delete that one as it's the first stop... They'll just have to lean that for now.
Indeed. I think I won't try to solve that one at present. The fill is a mixed style fill so seeing it as that on the fill icon is acceptable I think?
|
Joeclinton1
left a comment
Joeclinton1
left a comment
There was a problem hiding this comment.
- I like the solution to clicking outside the starting node. Clever.
- The new addon description is good.
- The reasoning for not doing the double click to edit is fine.
- The reasoning for not doing a tooltip is fine. And double clicking to remove is nice.
I do not think I can approve it with the glaring bugs of a new shape having an empty fill and the wrong preview. @Samq64 since you've been requested a reviewer on this what is your opinion? I'm going to give it a try myself to see how hard to achieve it is. If it's easy obviously I'll want it included. If I can't do it with reasonable effort then perhaps we can overlook this flaw.
Both were caused by the CHANGE_SELECTED_ITEMS listener being gated behind the colour picker overlay. Now it runs as soon as paper.js is cached (eagerly at startup) and infers the gradient type on first selection, so Redux stays clean regardless of overlay state.
|
I made the changes that fix the two bugs I mentioned. There is still a bug in that the preview is a simplified two stop version unsure whether to fix that too, as requires dom manipulation, due to a lack of support in the redux. |
|
So sorry for the delay. I got hugely busy. In the interest of moving this along I have chosen to replace my code with yours. Which is what the Adopt griffpatch paint-gradient-editor refactor commit does. Yours still leaves a "god file" userscript and continues to pass around state but how best to do this was subjective, and I trust your judgment so feel comfortable just using yours. Yours handles the GradientOverlay split better also. On top of your code I added the changes to use the tinycolor dependency. Because we load that anyway we might aswell use it. I've tested everything and am happy to approve it, if you're happy with it. (also if you're not happy with the choice to copy the code ontop of the changes leaving the history of the 5 now redundant commits, we can also do a force push clearing all the history as if I never did those commits, this will make the blame correctly attribute it to you) |
|
Yes, I'm all good with that - I agree that I'm still not 100% happy with
the modularization, but there reaches a point where I don't have time to
invest in working out that perfect split :) It's win some lose some.
If you are happy, then great. I'm happy enough! Thanks a lot for your help.
…On Fri, 17 Apr 2026 at 01:26, Joe Clinton ***@***.***> wrote:
*Joeclinton1* left a comment (ScratchAddons/ScratchAddons#8918)
<#8918 (comment)>
So sorry for the delay. I got hugely busy. In the interest of moving along
I have chosen to replace my code with yours. Which is what the Adopt
griffpatch paint-gradient-editor refactor
<8209f83>
commit does. Yours still leaves a "god file" userscript and continues to
pass around state but how best to do this was subjective, and I trust your
judgment so feel comfortable just using yours. Yours handles the
GradientOverlay split better also.
On top of your code I added the changes to use the tinycolor dependency.
Because we load that anyway we might aswell use it.
I've tested everything and am happy to approve it, if you're happy with it.
—
Reply to this email directly, view it on GitHub
<#8918 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABTM3PWUMJY7KUROFNHQGB34WF23LAVCNFSM6AAAAACXCNGWTGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DENRUGM2DMMJYGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Joeclinton1
left a comment
Joeclinton1
left a comment
There was a problem hiding this comment.
In interest of pushing this along, since it passes all the tests, and the structure looks good enough, I am approving this!
To the next reviewer: I was focused on the macroscopic effects and overall structure's maintainability in this review pass, but you may want to look at the details. I didn't see anything that jumped out at me, but honestly i'm a bit rusty now on this whole js extension stuff.
There was a problem hiding this comment.
This was quite difficult to test, as I ended up discovering a lot of bugs in the vanilla editor! Luckily, there's only two things that I think are more broken than they should be:
Clicking 'swap' does not swap the two colours that are shown on either side of the swap button, instead it sets one of the end stops to the same colour as the one next to it
2026-04-19.22-40-33.mp4
It is possible to lie all stops right on top of each other such that the paint editor preview is different to what's on the stage. Is this intentional / a paper.js bug / a scratch-svg-renderer bug? Also, once I'm in this state, I can't move the stops at all. 
Other general comments:
- Is it intentional that the opacity slider is shown even when the opacity slider addon is disabled?
- I found the transparency of the color picker when using the eyedropper a bit confusing, because the colour preview is behind the transparent colour picker window, but the transparency is so great that I thought there was some strange visual artifact going on with the eyedropper preview. I personally would prefer for the colour picker to be slightly less transparent, or for the eyedropper preview circle to appear in front of the colour picker. Not a big deal at all though
Seems to work well otherwise, modulo vanilla scratch bugs. My few comments on the code are very very minor. I'm a little confused by the complexity of the userscript initialisation - i.e. why it is "runAtComplete": false but then proceeds to wait for lots of things (redux, scratchClass) to load - but I trust that the complexity is necessary.
… into feature/paint-gradient-editor
…nto feature/paint-gradient-editor
When scratch-paint swaps gradient colours it reverses the 2-stop gradient in paper.js and fires CHANGE_SELECTED_ITEMS. The wipe-branch handler was keeping c1 from cache in all cases, so after a swap the extra stops were reinstated with the wrong outer colours. Fix: detect swap vs gradient-type-switch by checking whether the old c0 appears at paper stop[last] after the wipe. On swap, reverse the cached c0/c1 and mirror extra-stop offsets. On a type-switch, paper[last] is an extra stop colour so keep c1 from cache and only update c0.
…n while active - Remove opacity fade (0.15) and pointer-events:none applied to the stop-colour picker popup during eyedropper mode — popup stays fully visible and interactive while the colour picker is active - Add .sa-eyedropper-active CSS class (box-shadow glow matching Scratch's own eyedropper button style) toggled via classList in JS instead of inline styles
Arrghh!!! - Regression, I must have broken that. It's now fixed in the next commit I make.
Yeah, this is intentionally done. I don't like it when you can't use the stops because they are overlapping. So I made the gradient stop handles have a min distance from each other where they actually are considered touching. This does mean the actual gradient is slightly out of sync with the stop handles, but I think it's acceptable for the functional benefit. You can see exactly what you're getting anyhow.
Yes.
So, "runAtComplete: false" is needed so our Redux listeners are registered before React mounts the paint editor - if we waited until page load we'd miss the initial CHANGE_SELECTED_ITEMS dispatch and start out of sync. But, running that early means Redux, paper.js, and Scratch's hashed CSS class names aren't available yet, so we have to defer the parts that need those until they show up. The async waits are just bridging that gap between "early enough to catch events" and "late enough for dependencies to exist". I'm going to go through your other code notes before I push any of my fixes to this pull request |
- Replace existing?._setColor check with plain existing — _setColor is always set when the element exists - Remove dead existing?.remove() that followed an early return - Replace _close?.() with _close() — _close is always set when the element exists
…helper - Gradient overlay and stop picker now work when the RESHAPE/shaping tool is active: selected sub-paths inside a group have a Group parent, not a Layer parent, so they were silently excluded by every filter. Fix: accept parent instanceof Layer OR Group (groups are exactly one level deep in scratch-paint). - Extract the repeated filter predicate into selectedShapes(paper) in new paper-utils.js, replacing 10 inline occurrences across overlay.js, state-handlers.js, and userscript.js. - selectedShapes() returns [] when paper is null so callers are safe without pre-checking.
|
Found a new issue while testing - the new gradient editor did not appear when in shaping mode. I've fixed this now so you can easily target individual shapes within a group to change their gradient fills without ungrouping first (using the shaping tool) 
|
pufferfish101007
left a comment
pufferfish101007
left a comment
There was a problem hiding this comment.
Looks good to me. I don't think my approval counts toward anything but you can have it anyway.
DNin01
left a comment
DNin01
left a comment
There was a problem hiding this comment.
It feels pretty robust until you start using the paint bucket.
- The selected color sometimes resets or goes out of sync.
- Using the paint bucket inconsistently sets the colors of objects just by hovering over them.
- Editing the colors of a selection of multiple different gradients is weird.
multistop.gradient.bugs.mp4
DNin01
added
status: needs review
Co-authored-by: DNin01 <[email protected]>
Co-authored-by: DNin01 <[email protected]>
Joeclinton1
dismissed
their stale review
Too many changes since my review, need to re-review
|
Note that it is possible to attribute Claude Code, just remember to add this to the commit message when merging, filling in the model used (e.g., Sonnet 4.6): Just thinking, it's not a bad thing to disclose. |
- Fix fill tool to correctly copy the last-selected shape's exact fill (solid or gradient, including radial/linear axis and extra stops) instead of scratch-paint's sticky last-used gradient type. - Add a Shift-key eyedropper while the fill tool is active: hover any shape's fill or outline to preview its colour (gradient stops included) in the fill palette, click to pick it. - Document the new eyedropper in addon.json. - Deduplicate shared gradient-angle math and canvas/DOM lookups across userscript.js and state-handlers.js into gradient-coords.js.
|
Pushed a follow-up commit with fixes/additions to the Fill tool workflow:
Commit: e19a957 |
|
The idea is awesome |
8 participants
Resolves
Will close #4953
Changes
Adds a new Gradient stop editor addon to the vector costume editor.
When a linear or radial gradient fill or stroke is active, a gradient bar is injected directly onto the canvas as a live SVG overlay. The overlay is positioned and scaled to match the current paper.js view (including zoom and pan) and updates via a
requestAnimationFrameloop.How to use
Select a shape with a gradient fill or stroke and open its colour picker. A line with draggable handles appears on the canvas showing the gradient axis.
Fill (paint bucket) tool
Reason for changes
The built-in Scratch gradient tool only lets you set the two end colours. There is no way to reposition the gradient stops, add intermediate stops, adjust the midpoint bias, or shift the gradient axis without leaving the colour picker. This addon exposes those controls directly on the canvas, making gradient editing practical for complex artwork.
The Fill tool additions round out the workflow: they let you carry a complex, hand-tuned gradient (or a plain colour) from one shape to another without re-building it stop-by-stop, matching the eyedropper-style workflow other paint tools offer.
Tests
Tested in Chrome: