github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-05-14 20:43:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Fix several issues with color picker (#9558)

Browse Source
This commit is contained in:
Alonso Torres 2026-05-13 15:54:05 +02:00 committed by GitHub
parent b125c2cabb
commit c65b24495b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 157 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.gitignore vendored
View File

@ -86,3 +86,6 @@
/**/.yarn/*
/.pnpm-store
/.vscode
/.idea
/.claude
/.playwright-mcp

7
frontend/src/app/main/ui/workspace/colorpicker.cljs
View File

@ -376,7 +376,12 @@
;; Initialize colorpicker state
(mf/with-effect []
(st/emit! (dc/initialize-colorpicker on-change active-fill-tab))
(partial st/emit! (dc/finalize-colorpicker)))
;; Always deactivate picking mode on unmount so that :picking-color? never
;; stays true if the modal closes for any reason other than the normal
;; pointer-up path (e.g. ESC, navigation, programmatic hide).
(fn []
(st/emit! (dc/stop-picker)
(dc/finalize-colorpicker))))
;; Update colorpicker with external color changes
(mf/with-effect [data]

213
frontend/src/app/main/ui/workspace/viewport/pixel_overlay.cljs
View File

@ -53,26 +53,22 @@
new-canvas))))))))
(defn process-pointer-move
[viewport-node canvas canvas-image-data zoom-view-context client-x client-y]
[viewport-node canvas canvas-image-data zoom-view-context last-picked-color client-x client-y]
(when-let [image-data (mf/ref-val canvas-image-data)]
(when-let [zoom-view-node (dom/get-element "picker-detail")]
(when-not (mf/ref-val zoom-view-context)
(mf/set-ref-val! zoom-view-context (.getContext zoom-view-node "2d")))
(let [canvas-width 260
(let [canvas-width 260
canvas-height 140
{brx :left bry :top} (dom/get-bounding-rect viewport-node)
x (mth/floor (- client-x brx))
y (mth/floor (- client-y bry))
zoom-context (mf/ref-val zoom-view-context)
offset (* (+ (* y (unchecked-get image-data "width")) x) 4)
rgba (unchecked-get image-data "data")
img-width (unchecked-get image-data "width")
img-height (unchecked-get image-data "height")
r (obj/get rgba (+ 0 offset))
g (obj/get rgba (+ 1 offset))
b (obj/get rgba (+ 2 offset))
a (obj/get rgba (+ 3 offset))
zoom-context (mf/ref-val zoom-view-context)
sx (- x 32)
sy (if (cfg/check-browser? :safari) y (- y 17))
@ -82,13 +78,27 @@
dy 0
dw canvas-width
dh canvas-height]
(when (obj/get zoom-context "imageSmoothingEnabled")
(obj/set! zoom-context "imageSmoothingEnabled" false))
(.clearRect zoom-context 0 0 canvas-width canvas-height)
(.drawImage zoom-context canvas sx sy sw sh dx dy dw dh)
(js/requestAnimationFrame
(fn []
(st/emit! (dwc/pick-color [r g b a]))))))))
;; Only pick color when cursor is within canvas bounds to avoid garbage pixels
(when (and (>= x 0) (< x img-width) (>= y 0) (< y img-height))
(let [offset (* (+ (* y img-width) x) 4)
rgba (unchecked-get image-data "data")
r (obj/get rgba (+ 0 offset))
g (obj/get rgba (+ 1 offset))
b (obj/get rgba (+ 2 offset))
a (obj/get rgba (+ 3 offset))
color [r g b a]]
;; Store latest color synchronously so the click handler always reads
;; the correct pixel even before the rAF fires (fixes race condition)
(mf/set-ref-val! last-picked-color color)
(js/requestAnimationFrame
(fn []
(st/emit! (dwc/pick-color color))))))))))
(mf/defc pixel-overlay
@ -103,8 +113,14 @@
canvas-context (.getContext canvas "2d" #js {:willReadFrequently true})
canvas-image-data (mf/use-ref nil)
zoom-view-context (mf/use-ref nil)
canvas-ready (mf/use-state false)
initial-mouse-pos (mf/use-state {:x 0 :y 0})
;; Holds the last successfully picked [r g b a] synchronously so that
;; the pointer-down handler always has the current pixel, regardless of
;; whether the rAF-deferred store update has fired yet.
last-picked-color (mf/use-ref nil)
;; Use a ref (not state) so tracking the cursor doesn't cause re-renders.
;; Updated by both on-mouse-enter and a document-level pointermove listener
;; so that the position is always current when the canvas first becomes ready.
initial-mouse-pos (mf/use-ref {:x 0 :y 0})
update-str (rx/subject)
handle-keydown
@ -121,8 +137,15 @@
(fn [event]
(dom/prevent-default event)
(dom/stop-propagation event)
(st/emit! (dwu/start-undo-transaction :mouse-down-picker)
(dwc/pick-color-select true (kbd/shift? event)))))
;; Emit pick-color synchronously with the latest pixel colour before
;; pick-color-select, so the colorpicker effect never sees a stale value.
(let [color (mf/ref-val last-picked-color)]
(if (some? color)
(st/emit! (dwu/start-undo-transaction :mouse-down-picker)
(dwc/pick-color color)
(dwc/pick-color-select true (kbd/shift? event)))
(st/emit! (dwu/start-undo-transaction :mouse-down-picker)
(dwc/pick-color-select true (kbd/shift? event)))))))
handle-pointer-up-picker
(mf/use-callback
@ -143,16 +166,20 @@
:result "image-bitmap"}]
(->> (fonts/render-font-styles-cached fonts)
(rx/map (fn [styles]
(assoc result
:styles styles)))
(assoc result :styles styles)))
(rx/mapcat thr/render-node)
(rx/subs! (fn [image-bitmap]
(.drawImage canvas-context image-bitmap 0 0)
(let [width (unchecked-get canvas "width")
height (unchecked-get canvas "height")
image-data (.getImageData canvas-context 0 0 width height)]
image-data (.getImageData canvas-context 0 0 width height)
;; Read current mouse position from ref so the zoom
;; is populated immediately even without a mouse-move.
{mx :x my :y} (mf/ref-val initial-mouse-pos)]
(mf/set-ref-val! canvas-image-data image-data)
(reset! canvas-ready true))))))))
(process-pointer-move viewport-node canvas canvas-image-data
zoom-view-context last-picked-color
mx my))))))))
handle-svg-change
(mf/use-callback
@ -163,19 +190,28 @@
(mf/use-callback
(mf/deps viewport-node)
(fn [event]
(let [x (.-clientX event)
y (.-clientY event)]
(reset! initial-mouse-pos {:x x
:y y}))))
(mf/set-ref-val! initial-mouse-pos
{:x (.-clientX event)
:y (.-clientY event)})))
handle-pointer-move-picker
(mf/use-callback
(mf/deps viewport-node)
(fn [event]
(process-pointer-move viewport-node canvas canvas-image-data zoom-view-context (.-clientX event) (.-clientY event))))]
(process-pointer-move viewport-node canvas canvas-image-data zoom-view-context
last-picked-color (.-clientX event) (.-clientY event))))]
(when (obj/get canvas-context "imageSmoothingEnabled")
(obj/set! canvas-context "imageSmoothingEnabled" false))
;; Move focus to the overlay div on mount so the eyedropper button loses
;; :focus styling immediately. Without this, prevent-default on pointer-down
;; keeps focus on the button and it looks "selected" even after picking.
(mf/use-effect
(fn []
(when-let [node (dom/get-element "pixel-overlay")]
(.focus node))))
(mf/use-effect
(fn []
(let [listener (events/listen ug/document "keydown" handle-keydown)]
@ -202,12 +238,17 @@
;; Disconnect on unmount
#(.disconnect observer))))
;; Track the cursor position at document level so initial-mouse-pos is always
;; current when the canvas first becomes ready — even when the picker is opened
;; via the "i" shortcut and the cursor hasn't entered/moved over the overlay yet.
(mf/use-effect
(mf/deps viewport-node @canvas-ready)
(fn []
(when canvas-ready
(let [{:keys [x y]} @initial-mouse-pos]
(process-pointer-move viewport-node canvas canvas-image-data zoom-view-context x y)))))
(let [listener (events/listen ug/document "pointermove"
(fn [e]
(mf/set-ref-val! initial-mouse-pos
{:x (.-clientX e)
:y (.-clientY e)})))]
#(events/unlistenByKey listener))))
[:div {:id "pixel-overlay"
:tab-index 0
@ -218,12 +259,13 @@
:on-mouse-enter handle-mouse-enter}]))
(defn process-pointer-move-wasm [viewport-node canvas canvas-image-data zoom-view-context client-x client-y]
(defn process-pointer-move-wasm
[viewport-node canvas canvas-image-data zoom-view-context last-picked-color client-x client-y]
(when-let [image-data (mf/ref-val canvas-image-data)]
(when-let [zoom-view-node (dom/get-element "picker-detail")]
(when-not (mf/ref-val zoom-view-context)
(mf/set-ref-val! zoom-view-context (.getContext zoom-view-node "2d")))
(let [zoom-view-width 260
(let [zoom-view-width 260
zoom-view-height 140
{brx :left bry :top} (dom/get-bounding-rect viewport-node)
x (mth/floor (- client-x brx))
@ -232,31 +274,39 @@
canvas-x (* x wasm.api/dpr)
canvas-y (* y wasm.api/dpr)
zoom-context (mf/ref-val zoom-view-context)
;; the image-data we have is an array of pixels, starting from the
;; bottom-left corner; so we need to calculate the offset accordingly
inverted-y (- (.-height image-data) canvas-y)
offset (* (+ (* inverted-y (.-width image-data)) canvas-x) 4)
rgba (.-data image-data)
img-width (.-width image-data)
img-height (.-height image-data)
r (obj/get rgba (+ 0 offset))
g (obj/get rgba (+ 1 offset))
b (obj/get rgba (+ 2 offset))
a (obj/get rgba (+ 3 offset))
zoom-context (mf/ref-val zoom-view-context)
sx (- canvas-x 32)
sy (if (cfg/check-browser? :safari) canvas-y (- canvas-y 17))
sw 65
sh 35]
(when (obj/get zoom-context "imageSmoothingEnabled")
(obj/set! zoom-context "imageSmoothingEnabled" false))
(.clearRect zoom-context 0 0 zoom-view-width zoom-view-height)
(.drawImage zoom-context canvas sx sy sw sh 0 0 zoom-view-width zoom-view-height)
;; FIXME: this is throttled to avoid getting stuck in an inifinite react
;; update loop. We should fix the global state instead.
(js/requestAnimationFrame
(fn []
(st/emit! (dwc/pick-color [r g b a]))))))))
;; Only pick color when cursor is within canvas bounds to avoid garbage pixels
(when (and (>= canvas-x 0) (< canvas-x img-width) (>= canvas-y 0) (< canvas-y img-height))
(let [;; image-data pixels start from the bottom-left corner; invert y accordingly
inverted-y (- img-height canvas-y)
offset (* (+ (* inverted-y img-width) canvas-x) 4)
rgba (.-data image-data)
r (obj/get rgba (+ 0 offset))
g (obj/get rgba (+ 1 offset))
b (obj/get rgba (+ 2 offset))
a (obj/get rgba (+ 3 offset))
color [r g b a]]
;; Store latest color synchronously so the click handler always reads
;; the correct pixel even before the rAF fires (fixes race condition)
(mf/set-ref-val! last-picked-color color)
;; rAF throttles state updates to avoid an infinite React re-render loop
(js/requestAnimationFrame
(fn []
(st/emit! (dwc/pick-color color))))))))))
(mf/defc pixel-overlay-wasm*
{::mf/wrap-props false}
@ -266,7 +316,14 @@
canvas-context (mf/use-ref nil)
canvas-image-data (mf/use-ref nil)
zoom-view-context (mf/use-ref nil)
initial-mouse-pos (mf/use-state {:x 0 :y 0})
;; Holds the last successfully picked [r g b a] synchronously so that
;; the pointer-down handler always has the current pixel, regardless of
;; whether the rAF-deferred store update has fired yet.
last-picked-color (mf/use-ref nil)
;; Use a ref (not state) so tracking the cursor doesn't cause re-renders.
;; Updated by both on-mouse-enter and a document-level pointermove listener
;; so that the position is always current when the canvas first becomes ready.
initial-mouse-pos (mf/use-ref {:x 0 :y 0})
update-str (rx/subject)
handle-keydown
@ -283,8 +340,15 @@
(fn [event]
(dom/prevent-default event)
(dom/stop-propagation event)
(st/emit! (dwu/start-undo-transaction :mouse-down-picker)
(dwc/pick-color-select true (kbd/shift? event)))))
;; Emit pick-color synchronously with the latest pixel colour before
;; pick-color-select, so the colorpicker effect never sees a stale value.
(let [color (mf/ref-val last-picked-color)]
(if (some? color)
(st/emit! (dwu/start-undo-transaction :mouse-down-picker)
(dwc/pick-color color)
(dwc/pick-color-select true (kbd/shift? event)))
(st/emit! (dwu/start-undo-transaction :mouse-down-picker)
(dwc/pick-color-select true (kbd/shift? event)))))))
handle-pointer-up-picker
(mf/use-callback
@ -300,12 +364,18 @@
(mf/deps canvas-context)
(fn []
(when-let [canvas-context (mf/ref-val canvas-context)]
(let [width (.-width canvas)
(let [width (.-width canvas)
height (.-height canvas)
buffer (js/Uint8ClampedArray. (* width height 4))
_ (.readPixels canvas-context 0 0 width height (.-RGBA canvas-context) (.-UNSIGNED_BYTE canvas-context) buffer)
image-data (js/ImageData. buffer width height)]
(mf/set-ref-val! canvas-image-data image-data)))))
buffer (js/Uint8ClampedArray. (* width height 4))
_ (.readPixels canvas-context 0 0 width height (.-RGBA canvas-context) (.-UNSIGNED_BYTE canvas-context) buffer)
image-data (js/ImageData. buffer width height)
;; Read current mouse position from ref so the zoom
;; is populated immediately even without a mouse-move.
{mx :x my :y} (mf/ref-val initial-mouse-pos)]
(mf/set-ref-val! canvas-image-data image-data)
(process-pointer-move-wasm viewport-node canvas canvas-image-data
zoom-view-context last-picked-color
mx my)))))
handle-canvas-changed
(mf/use-callback
@ -316,25 +386,33 @@
(mf/use-callback
(mf/deps viewport-node)
(fn [event]
(let [x (.-clientX event)
y (.-clientY event)]
(reset! initial-mouse-pos {:x x
:y y}))))
(mf/set-ref-val! initial-mouse-pos
{:x (.-clientX event)
:y (.-clientY event)})))
handle-pointer-move-picker
(mf/use-callback
(mf/deps viewport-node)
(fn [event]
(process-pointer-move-wasm viewport-node canvas canvas-image-data zoom-view-context (.-clientX event) (.-clientY event))))]
(process-pointer-move-wasm viewport-node canvas canvas-image-data zoom-view-context last-picked-color (.-clientX event) (.-clientY event))))]
(mf/use-effect
(mf/deps canvas)
(fn []
(let [context (.getContext canvas "webgl2" #js {:willReadFrequently true, :preserveDrawingBuffer true})]
(let [context (.getContext canvas "webgl2" #js {:willReadFrequently true :preserveDrawingBuffer true})]
(mf/set-ref-val! canvas-context context))))
;; Move focus to the overlay div on mount so the eyedropper button loses
;; :focus styling immediately. Without this, prevent-default on pointer-down
;; keeps focus on the button and it looks "selected" even after picking.
(mf/use-effect
(fn []
(when-let [node (dom/get-element "pixel-overlay")]
(.focus node))))
(mf/use-effect
(fn []
(let [listener (events/listen ug/document "keydown" handle-keydown)]
(let [listener (events/listen ug/document "keydown" handle-keydown)]
#(events/unlistenByKey listener))))
(mf/use-effect
@ -350,12 +428,17 @@
(fn []
(.removeEventListener ug/document "penpot:wasm:render" handle-canvas-changed)))
;; Track the cursor position at document level so initial-mouse-pos is always
;; current when the canvas first becomes ready — even when the picker is opened
;; via the "i" shortcut and the cursor hasn't entered/moved over the overlay yet.
(mf/use-effect
(mf/deps viewport-node canvas canvas-image-data zoom-view-context)
(fn []
(when (some? canvas)
(let [{:keys [x y]} @initial-mouse-pos]
(process-pointer-move-wasm viewport-node canvas canvas-image-data zoom-view-context x y)))))
(let [listener (events/listen ug/document "pointermove"
(fn [e]
(mf/set-ref-val! initial-mouse-pos
{:x (.-clientX e)
:y (.-clientY e)})))]
#(events/unlistenByKey listener))))
[:div {:id "pixel-overlay"
:tab-index 0