github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-05-09 01:58:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Skip add-recent-color when colorpicker has no completed color (#9251)

Browse Source 
Closing the fill dialog while an image-fill upload is still in flight
(or while a gradient is mid-edit) leaves the colorpicker's
current-color with only :opacity — no :image, :gradient, or :color.
update-colorpicker-color's WatchEvent then constructed
`(add-recent-color partial)`, which runs the value through
`clr/check-color` and threw "expected valid color". The user saw an
Internal Assertion Error toast and lost the in-flight upload.

The existing `ignore-color?` guard reads `:type` from the *output* of
`get-color-from-colorpicker-state` — but that helper strips :type from
its result, so the guard never actually fires. Add a schema-based gate
(same validator add-recent-color itself uses) right before `rx/of`, so
a partial selection is silently dropped instead of crashing the
workspace. Behaviour for fully-valid colors is unchanged.

Tests cover three cases: (1) a partial image-tab state with only
:opacity returns nil from watch (was: throws); (2) the same partial
shape on the color tab also returns nil — pinning down that the prior
:type guard wouldn't have caught it; (3) a fully-populated plain color
still produces a watch observable so the guard isn't over-eager.

Closes #8443

Co-authored-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
Jeff 2026-05-08 17:37:12 +02:00 committed by GitHub
parent ea24445c2c
commit f5b38a5025
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 45 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
frontend/src/app/main/data/workspace/colors.cljs
View File

@ -1048,7 +1048,15 @@
(when-let [color (-> state
(select-keys [:image :gradient :color :opacity])
(not-empty))]
(rx/of (add-recent-color color))))))))
;; Closing the dialog while an image-fill upload is still in
;; flight (or a gradient is mid-edit) leaves the colorpicker
;; with a partial selection — opacity-only, or with stops not
;; yet committed. ``add-recent-color`` runs the value through
;; ``check-color`` and asserts; gate on the same schema here
;; so the partial value is silently dropped instead of crashing
;; the workspace.
(when (clr/valid-color? color)
(rx/of (add-recent-color color)))))))))
(defn update-colorpicker-gradient
[changes]

36
frontend/test/frontend_tests/data/workspace_colors_test.cljs
View File

@ -46,3 +46,39 @@
(ptk/event? (dwc/add-fill #{uuid/zero} color3 1))))
(t/is (thrown? js/Error
(ptk/event? (dwc/add-fill #{uuid/zero} color4 1))))))
(t/deftest update-colorpicker-color-skips-add-recent-on-incomplete-image-state
;; Regression for https://github.com/penpot/penpot/issues/8443.
;;
;; Closing the fill dialog while the image upload is still in flight leaves
;; the colorpicker's current-color with only :opacity (no :image, :gradient,
;; or :color). Before the guard, ptk/watch eagerly built (add-recent-color
;; partial), which calls (clr/check-color partial) and threw an "expected
;; valid color" assertion that surfaced as an Internal Assertion Error toast.
(let [partial-image-state {:colorpicker {:type :image
:current-color {:opacity 1}}}
event (dwc/update-colorpicker-color {} true)]
(t/is (nil? (ptk/watch event partial-image-state nil)))))
(t/deftest update-colorpicker-color-skips-add-recent-when-only-opacity-on-color-type
;; Same incomplete-state shape, but the colorpicker is on the plain-color tab
;; (e.g. the user clicked elsewhere before the picker had a chance to commit
;; a hex). The existing :type-and-:color-nil guard sits on the colorpicker
;; map's :type — but get-color-from-colorpicker-state strips :type from its
;; output, so that guard never fires. The schema-based guard catches it.
(let [colorless-state {:colorpicker {:type :color
:current-color {:opacity 1}}}
event (dwc/update-colorpicker-color {} true)]
(t/is (nil? (ptk/watch event colorless-state nil)))))
(t/deftest update-colorpicker-color-still-emits-recent-for-valid-plain-color
;; Sanity check: a fully-populated plain color still produces a watch
;; observable (the rx/of branch is reached) so we know the guard isn't
;; over-eager and silently dropping legitimate colors.
(let [valid-state {:colorpicker {:type :color
:current-color {:color "#ff0000" :opacity 1}}}
event (dwc/update-colorpicker-color {} true)]
(t/is (some? (ptk/watch event valid-state nil)))))