From 888dbef07094eb5dde1cd63916ab51777be5e848 Mon Sep 17 00:00:00 2001 From: Roman Vyakhirev Date: Fri, 27 Mar 2026 16:15:06 +0100 Subject: [PATCH 1/3] fix: improve overall user experience with onChange action, fix buggy logic --- .../color-picker-web/CHANGELOG.md | 4 ++ .../src/components/ColorPicker.tsx | 50 +++++++++++++------ 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/packages/pluggableWidgets/color-picker-web/CHANGELOG.md b/packages/pluggableWidgets/color-picker-web/CHANGELOG.md index a405a1dade..358d2b7c22 100644 --- a/packages/pluggableWidgets/color-picker-web/CHANGELOG.md +++ b/packages/pluggableWidgets/color-picker-web/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +### Fixed + +- We fixed an issue with On change action not triggering in some cases. + ## [2.1.5] - 2026-03-06 ### Fixed diff --git a/packages/pluggableWidgets/color-picker-web/src/components/ColorPicker.tsx b/packages/pluggableWidgets/color-picker-web/src/components/ColorPicker.tsx index db296cc164..6c88656f7a 100644 --- a/packages/pluggableWidgets/color-picker-web/src/components/ColorPicker.tsx +++ b/packages/pluggableWidgets/color-picker-web/src/components/ColorPicker.tsx @@ -1,6 +1,5 @@ -import { ReactElement, useCallback, useEffect, useState } from "react"; -import { Alert } from "@mendix/widget-plugin-component-kit/Alert"; -import { DefaultColorsType, FormatEnum, ModeEnum, TypeEnum } from "../../typings/ColorPickerProps"; +import classNames from "classnames"; +import { ReactElement, useCallback, useEffect, useMemo, useState, useRef } from "react"; import { BlockPickerProps, ChromePickerProps, @@ -15,10 +14,12 @@ import { SwatchesPickerProps, TwitterPickerProps } from "react-color"; -import classNames from "classnames"; +import { Alert } from "@mendix/widget-plugin-component-kit/Alert"; +import { debounce } from "@mendix/widget-plugin-platform/utils/debounce"; +import { DefaultColorsType, FormatEnum, ModeEnum, TypeEnum } from "../../typings/ColorPickerProps"; import { getColorPicker, parseColor, validateColorFormat, validateProps } from "../utils"; -import { Input } from "./Input"; import { Button } from "./Button"; +import { Input } from "./Input"; export interface ColorPickerProps { id: string; @@ -53,18 +54,33 @@ export const ColorPicker = (props: ColorPickerProps): ReactElement => { rgb: "rgb(255,255,255)", rgba: "rgb(255,255,255,1)" }; - const { type, mode, disabled, defaultColors, color, format, invalidFormatMessage, onColorChange } = props; + const { + type, + mode, + disabled, + defaultColors, + color, + format, + invalidFormatMessage, + onColorChange, + onChange: onColorChangeComplete + } = props; const ColorElement = getColorPicker(type); const [hidden, setHidden] = useState(mode !== "inline"); - const [currentColor, setCurrentColor] = useState(color); + const currentColor = useRef(color); const [alertMessage, setAlertMessage] = useState(); + const [completeColorChange, abortCompleteColorChange] = useMemo(() => { + return debounce(onColorChangeComplete, 500); + }, [onColorChangeComplete]); + const submitColor = useCallback( (color: string): void => { - setCurrentColor(color); + currentColor.current = color; onColorChange(color); + abortCompleteColorChange(); }, - [onColorChange] + [onColorChange, abortCompleteColorChange] ); const validateColor = (colorValue: string): void => { @@ -93,7 +109,7 @@ export const ColorPicker = (props: ColorPickerProps): ReactElement => { ); const renderInput = (): ReactElement => { - const colorValue = currentColor || color; + const colorValue = currentColor.current || color; return ( { ); }; - const onChangeComplete = (color: ColorState): void => { - if (currentColor !== parseColor(color, format)) { - props.onChange(); - } - }; + const onChangeComplete = useCallback( + (color: ColorState): void => { + if (currentColor.current === parseColor(color, format)) { + completeColorChange(); + } + }, + [format, completeColorChange] + ); + const renderButton = (): ReactElement => { return + + + ); + }; + + return { + SketchPicker: createMockPicker("sketch-picker"), + ChromePicker: createMockPicker("chrome-picker"), + BlockPicker: createMockPicker("block-picker"), + GithubPicker: createMockPicker("github-picker"), + TwitterPicker: createMockPicker("twitter-picker"), + CirclePicker: createMockPicker("circle-picker"), + HuePicker: createMockPicker("hue-picker"), + SliderPicker: createMockPicker("slider-picker"), + CompactPicker: createMockPicker("compact-picker"), + MaterialPicker: createMockPicker("material-picker"), + SwatchesPicker: createMockPicker("swatches-picker") + }; +}); + +describe("ColorPicker – debounced onChange behavior", () => { + /** + * color="#FF0000" matches the hex value emitted by the mock picker so that + * the guard in onChangeComplete (`currentColor.current === parseColor(color, format)`) + * passes and completeColorChange is actually scheduled. + */ + const baseProps: ColorPickerProps = { + id: "color-picker", + name: "color picker", + color: "#FF0000", + disabled: false, + defaultColors: [], + format: "hex", + mode: "inline", + type: "sketch", + onChange: jest.fn(), + onColorChange: jest.fn() + }; + + let user: UserEvent; + + beforeEach(() => { + jest.useFakeTimers(); + // advanceTimers keeps userEvent in sync with fake timers + user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + baseProps.onChange = jest.fn(); + baseProps.onColorChange = jest.fn(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it("does not call onChange immediately after onChangeComplete fires", async () => { + const { getByTestId } = render(); + + // onChange sets currentColor.current so the guard in onChangeComplete passes + await user.click(getByTestId("trigger-change")); + await user.click(getByTestId("trigger-change-complete")); + + // Debounce delay has not elapsed yet + expect(baseProps.onChange).not.toHaveBeenCalled(); + }); + + it("calls onChange once after the 500 ms debounce delay", async () => { + const { getByTestId } = render(); + + await user.click(getByTestId("trigger-change")); + await user.click(getByTestId("trigger-change-complete")); + + expect(baseProps.onChange).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(500); + + expect(baseProps.onChange).toHaveBeenCalledTimes(1); + }); + + it("calls onChange only once when multiple rapid color changes occur", async () => { + const { getByTestId } = render(); + + // Each iteration: onChange aborts the previous pending debounce and + // onChangeComplete schedules a new one. Only the last scheduled call + // survives. + for (let i = 0; i < 5; i++) { + await user.click(getByTestId("trigger-change")); + await user.click(getByTestId("trigger-change-complete")); + // Advance less than the debounce window so the timer never fires mid-loop + jest.advanceTimersByTime(100); + } + + // Let the last pending debounce fire + jest.advanceTimersByTime(500); + + expect(baseProps.onChange).toHaveBeenCalledTimes(1); + }); + + it("does not call onChange if abortCompleteColorChange is called before the delay elapses", async () => { + const { getByTestId } = render(); + + // Schedule a completeColorChange via onChangeComplete + await user.click(getByTestId("trigger-change")); + await user.click(getByTestId("trigger-change-complete")); + + // Advance partway through the debounce window + jest.advanceTimersByTime(200); + + // Triggering onChange calls submitColor → abortCompleteColorChange, + // cancelling the previously scheduled debounce + await user.click(getByTestId("trigger-change")); + + // Advance past the original debounce deadline + jest.advanceTimersByTime(500); + + expect(baseProps.onChange).not.toHaveBeenCalled(); + }); +});