From d09ca57373143c555676372e1688976dfe68fafa Mon Sep 17 00:00:00 2001 From: Ivan Starkov Date: Mon, 19 Dec 2022 18:17:36 +0300 Subject: [PATCH] Check mark in Combo Boxes can be lost or be incorrect in case of input changes (#652) closes #610 ## Before requesting a review - [ ] if the PR is WIP - use draft mode - [ ] made a self-review - [x] added inline comments where things may be not obvious (the "why", not "what") - [ ] what kind of review is needed? - [ ] conceptual - [ ] detailed - [x] with testing ## Test cases - [ ] step by step interaction description and what is expected to happen ## Before merging - [x] tested locally and on preview environment (preview dev login: 5de6) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio-designer/blob/main/apps/designer/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env.example` and the `designer/env-check.js` if mandatory --- .../features/props-panel/props-panel.tsx | 1 + .../controls/text/text-control.tsx | 11 +-- .../css-value-input.stories.tsx | 74 +++++++++++-------- .../css-value-input/css-value-input.tsx | 18 +++-- .../src/components/combobox.stories.tsx | 2 + .../design-system/src/components/combobox.tsx | 13 +++- 6 files changed, 74 insertions(+), 45 deletions(-) diff --git a/apps/designer/app/designer/features/props-panel/props-panel.tsx b/apps/designer/app/designer/features/props-panel/props-panel.tsx index 1aa0839712..c5038ca9cb 100644 --- a/apps/designer/app/designer/features/props-panel/props-panel.tsx +++ b/apps/designer/app/designer/features/props-panel/props-panel.tsx @@ -65,6 +65,7 @@ const Combobox = ({ } = useCombobox({ items: itemsProp, value, + selectedItem: value, itemToString: (item) => item ?? "", onItemSelect, }); diff --git a/apps/designer/app/designer/features/style-panel/controls/text/text-control.tsx b/apps/designer/app/designer/features/style-panel/controls/text/text-control.tsx index 7ef8947721..a46b45786c 100644 --- a/apps/designer/app/designer/features/style-panel/controls/text/text-control.tsx +++ b/apps/designer/app/designer/features/style-panel/controls/text/text-control.tsx @@ -23,7 +23,7 @@ export const TextControl = ({ const setValue = setProperty(styleConfig.property); - const [currentValue, setCurrentValue] = useState< + const [intermediateValue, setIntermediateValue] = useState< StyleValue | IntermediateStyleValue >(); @@ -37,23 +37,24 @@ export const TextControl = ({ ({ type: "keyword", value: item.name, }))} onChange={(styleValue) => { - setCurrentValue(styleValue); + setIntermediateValue(styleValue); if (styleValue.type !== "intermediate") { setValue(styleValue, { isEphemeral: true }); } }} - onPreview={(styleValue) => { + onHighlight={(styleValue) => { setValue(styleValue, { isEphemeral: true }); }} onChangeComplete={(styleValue) => { setValue(styleValue); - setCurrentValue(undefined); + setIntermediateValue(undefined); }} /> diff --git a/apps/designer/app/designer/features/style-panel/shared/css-value-input/css-value-input.stories.tsx b/apps/designer/app/designer/features/style-panel/shared/css-value-input/css-value-input.stories.tsx index e2f7ceccf2..98780f358c 100644 --- a/apps/designer/app/designer/features/style-panel/shared/css-value-input/css-value-input.stories.tsx +++ b/apps/designer/app/designer/features/style-panel/shared/css-value-input/css-value-input.stories.tsx @@ -1,4 +1,4 @@ -import React from "react"; +import * as React from "react"; import { Flex, TextField } from "@webstudio-is/design-system"; import type { StyleValue } from "@webstudio-is/css-data"; import { CssValueInput, type IntermediateStyleValue } from "./css-value-input"; @@ -10,17 +10,20 @@ export default { }; export const WithKeywords = () => { - const [value, setValue] = React.useState( - { - type: "keyword", - value: "auto", - } - ); + const [value, setValue] = React.useState({ + type: "keyword", + value: "auto", + }); + + const [intermediateValue, setIntermediateValue] = React.useState< + StyleValue | IntermediateStyleValue + >(); return ( { { type: "keyword", value: "fit-content" }, ]} onChange={(value) => { - setValue(value); + setIntermediateValue(value); }} - onPreview={(value) => { - action("onPreview")(value); + onHighlight={(value) => { + action("onHighlight")(value); }} onChangeComplete={(newValue) => { // on blur, select, enter etc. setValue(newValue); + setIntermediateValue(undefined); action("onChangeComplete")(newValue); }} /> @@ -43,17 +47,20 @@ export const WithKeywords = () => { }; export const WithIcons = () => { - const [value, setValue] = React.useState( - { - type: "keyword", - value: "space-around", - } - ); + const [value, setValue] = React.useState({ + type: "keyword", + value: "space-around", + }); + + const [intermediateValue, setIntermediateValue] = React.useState< + StyleValue | IntermediateStyleValue + >(); return ( { { type: "keyword", value: "space-between" }, ]} onChange={(newValue) => { - setValue(newValue); + setIntermediateValue(newValue); }} - onPreview={(value) => { - action("onPreview")(value); + onHighlight={(value) => { + action("onHighlight")(value); }} onChangeComplete={(newValue) => { // on blur, select, enter etc. setValue(newValue); + setIntermediateValue(undefined); action("onChangeComplete")(newValue); }} /> @@ -79,19 +87,22 @@ export const WithIcons = () => { }; export const WithUnits = () => { - const [value, setValue] = React.useState( - { - type: "unit", - value: 100, - unit: "px", - } - ); + const [value, setValue] = React.useState({ + type: "unit", + value: 100, + unit: "px", + }); + + const [intermediateValue, setIntermediateValue] = React.useState< + StyleValue | IntermediateStyleValue + >(); return ( { { type: "keyword", value: "fit-content" }, ]} onChange={(newValue) => { - setValue(newValue); + setIntermediateValue(newValue); }} - onPreview={(value) => { - action("onPreview")(value); + onHighlight={(value) => { + action("onHighlight")(value); }} onChangeComplete={(newValue) => { // on blur, select, enter etc. setValue(newValue); + setIntermediateValue(undefined); action("onChangeComplete")(newValue); }} /> @@ -114,8 +126,8 @@ export const WithUnits = () => { readOnly value={ value - ? value?.type === "intermediate" - ? value.value + value.unit + ? intermediateValue?.type === "intermediate" + ? intermediateValue.value + intermediateValue.unit : toValue(value) : "" } diff --git a/apps/designer/app/designer/features/style-panel/shared/css-value-input/css-value-input.tsx b/apps/designer/app/designer/features/style-panel/shared/css-value-input/css-value-input.tsx index 283a06f7f7..8625ce2ca7 100644 --- a/apps/designer/app/designer/features/style-panel/shared/css-value-input/css-value-input.tsx +++ b/apps/designer/app/designer/features/style-panel/shared/css-value-input/css-value-input.tsx @@ -186,11 +186,15 @@ type CssValueInputValue = StyleValue | IntermediateStyleValue; type CssValueInputProps = { property: StyleProperty; - value?: CssValueInputValue; + value: StyleValue | undefined; + intermediateValue: CssValueInputValue | undefined; + /** + * Selected item in the dropdown + */ keywords?: Array; onChange: (value: CssValueInputValue) => void; onChangeComplete: (value: StyleValue) => void; - onPreview: (value: StyleValue) => void; + onHighlight: (value: StyleValue) => void; }; /** @@ -227,11 +231,12 @@ type CssValueInputProps = { export const CssValueInput = ({ icon, property, - value = unsetValue, keywords = [], - onPreview, + onHighlight, ...props }: CssValueInputProps & { icon?: JSX.Element }) => { + const value = props.intermediateValue ?? props.value ?? unsetValue; + const onChange = (input: string) => { // We don't know what's inside the input, // preserve current unit value if exists @@ -263,6 +268,7 @@ export const CssValueInput = ({ } = useCombobox({ items: keywords, value, + selectedItem: props.value, itemToString: (item) => item === null ? "" @@ -277,11 +283,11 @@ export const CssValueInput = ({ }, onItemHighlight: (value) => { if (value == null) { - onPreview(unsetValue); + onHighlight(unsetValue); return; } if (value.type !== "intermediate") { - onPreview(value ?? unsetValue); + onHighlight(value ?? unsetValue); } }, }); diff --git a/packages/design-system/src/components/combobox.stories.tsx b/packages/design-system/src/components/combobox.stories.tsx index aeee375f66..198e4418af 100644 --- a/packages/design-system/src/components/combobox.stories.tsx +++ b/packages/design-system/src/components/combobox.stories.tsx @@ -23,6 +23,7 @@ export const Simple: ComponentStory = () => { placeholder="Select a fruit" items={items} value={value} + selectedItem={value} onItemSelect={(value) => { // If the value is cleared, we revert to previous state if (value !== null) { @@ -70,6 +71,7 @@ export const Complex: ComponentStory = () => { useCombobox({ items: ["Apple", "Banana", "Orange"], value, + selectedItem: value, itemToString: (item) => item ?? "", stateReducer, onItemSelect: (value) => { diff --git a/packages/design-system/src/components/combobox.tsx b/packages/design-system/src/components/combobox.tsx index c7872fe81b..12e7c35bc7 100644 --- a/packages/design-system/src/components/combobox.tsx +++ b/packages/design-system/src/components/combobox.tsx @@ -134,6 +134,7 @@ type UseComboboxProps = UseDownshiftComboboxProps & { items: Array; itemToString: (item: Item | null) => string; value: Item | null; // This is to prevent: "downshift: A component has changed the uncontrolled prop "selectedItem" to be controlled." + selectedItem: Item | undefined; onInputChange?: (value: string | undefined) => void; onItemSelect?: (value: Item | null) => void; onItemHighlight?: (value: Item | null) => void; @@ -149,6 +150,7 @@ export const comboboxStateChangeTypes = useDownshiftCombobox.stateChangeTypes; export const useCombobox = ({ items, value, + selectedItem, itemToString, onInputChange, onItemSelect, @@ -167,9 +169,10 @@ export const useCombobox = ({ ...rest, items: filteredItems, defaultHighlightedIndex: -1, - selectedItem: value, // Prevent downshift warning about switching controlled mode + selectedItem: selectedItem ?? null, // Prevent downshift warning about switching controlled mode stateReducer, itemToString, + inputValue: value ? itemToString(value) : undefined, onInputValueChange({ inputValue, type }) { if (type === comboboxStateChangeTypes.InputChange) { filter(inputValue); @@ -186,7 +189,7 @@ export const useCombobox = ({ }, }); - const { isOpen, getItemProps, highlightedIndex, selectedItem, getMenuProps } = + const { isOpen, getItemProps, highlightedIndex, getMenuProps } = downshiftProps; useEffect(() => { @@ -200,7 +203,9 @@ export const useCombobox = ({ return getItemProps({ highlighted: highlightedIndex === options.index, // We need to either deep compare objects here or use itemToString to get primitive types - selected: itemToString(selectedItem) === itemToString(options.item), + selected: + selectedItem !== undefined && + itemToString(selectedItem) === itemToString(options.item), key: options.id, ...options, }); @@ -238,6 +243,7 @@ export const Combobox = ({ items, value = null, name, + selectedItem, placeholder, itemToString, onItemSelect, @@ -254,6 +260,7 @@ export const Combobox = ({ } = useCombobox({ items, value, + selectedItem, itemToString, onItemSelect, onItemHighlight,