fix(ui-components): UITreeDropdown change callback being triggered tw…

…ice on first selection: (#2547) * fix: tree dropdown change callback called twice and with wrong value tree dropdown change callback called twice and with wrong value * test: missing expectation missing expectation * changelog changelog * fix: remove experimental code remove experimental code --------- Co-authored-by: Klaus Keller <[email protected]>
SAP · Nov 11, 2024 · 1c8b59f · 1c8b59f
1 parent 742d618
commit 1c8b59f
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 9 deletions.
diff --git a/.changeset/perfect-seals-try.md b/.changeset/perfect-seals-try.md
@@ -0,0 +1,7 @@
+---
+'@sap-ux/ui-components': patch
+---
+
+Fix UITreeDropdown change callback being triggered twice on first selection:
+1. First call with empty value
+2. Second call with selected value
diff --git a/packages/ui-components/src/components/UITreeDropdown/UITreeDropdown.tsx b/packages/ui-components/src/components/UITreeDropdown/UITreeDropdown.tsx
@@ -62,7 +62,6 @@ interface TreeItemInfo {
 }
 
 export interface UITreeDropdownState {
-    hasSelected: boolean;
     originalItems: IContextualMenuItem[];
     isHidden: boolean;
     value?: string;
@@ -117,6 +116,8 @@ export class UITreeDropdown extends React.Component<UITreeDropdownProps, UITreeD
     // Hold original value which should be stored when contextmenu opened
     // Restore can happend if user presses Escape on keyboard
     private originalValue?: string;
+    // Flag which indicates that value was selected in dropdown menu
+    private hasSelected: boolean;
     /**
      * Initializes component properties.
      *
@@ -126,7 +127,6 @@ export class UITreeDropdown extends React.Component<UITreeDropdownProps, UITreeD
         super(props);
         this.state = {
             query: '',
-            hasSelected: !!this.props.value,
             // value has to be set, otherwise react treats this as "uncontrolled" component
             // and displays warnings when value is set later on
             value: this.props.value ?? '',
@@ -139,6 +139,7 @@ export class UITreeDropdown extends React.Component<UITreeDropdownProps, UITreeD
             isMenuOpen: false,
             valueChanged: false
         };
+        this.hasSelected = !!this.props.value;
         this.toggleMenu = this.toggleMenu.bind(this);
         this.onWindowKeyDown = this.onWindowKeyDown.bind(this);
         this.handleCustomDownKey = this.handleCustomDownKey.bind(this);
@@ -260,9 +261,8 @@ export class UITreeDropdown extends React.Component<UITreeDropdownProps, UITreeD
      * @param {string} value
      */
     handleSelection = (value: string): void => {
-        this.setState({ hasSelected: true, value: value, valueChanged: false }, () =>
-            this.props.onParameterValueChange(value)
-        );
+        this.hasSelected = true;
+        this.setState({ value: value, valueChanged: false }, () => this.props.onParameterValueChange(value));
     };
     /**
      * Handle the keypress value.
@@ -429,8 +429,8 @@ export class UITreeDropdown extends React.Component<UITreeDropdownProps, UITreeD
     handleOnChangeValue = (event: React.FormEvent<HTMLInputElement | HTMLTextAreaElement>): void => {
         const query = event.target as HTMLInputElement;
 
+        this.hasSelected = false;
         this.setState((prevState) => ({
-            hasSelected: false,
             value: query.value,
             items: prevState.originalItems.filter((item) => this.filterElement(query.value, item)),
             query: query.value,
@@ -460,7 +460,7 @@ export class UITreeDropdown extends React.Component<UITreeDropdownProps, UITreeD
     handleDismiss = (event?: Event | React.MouseEvent<Element, MouseEvent> | React.KeyboardEvent): void => {
         if (event && 'key' in event && event.key === KEYBOARD_KEYS.Escape) {
             this.resetValue();
-        } else if (!this.state.hasSelected) {
+        } else if (!this.hasSelected) {
             this.props.onParameterValueChange('');
         }
 

diff --git a/packages/ui-components/stories/UITreedropdown.story.tsx b/packages/ui-components/stories/UITreedropdown.story.tsx
@@ -35,14 +35,18 @@ const data = [
 ];
 
 export const Basic = (): JSX.Element => {
+    const [changesCount, setChangesCount] = React.useState(0);
     const [value, setValue] = React.useState('');
-    const handleSelected = (value: any) => setValue(value);
+    const handleSelected = (value: any) => {
+        setChangesCount(changesCount + 1);
+        setValue(value);
+    };
 
     return (
         <div style={{ width: 300 }}>
             <UITreeDropdown
                 value={value}
-                label="Label"
+                label={`Label(value changed ${changesCount} times)`}
                 placeholderText="Select value"
                 items={data}
                 onParameterValueChange={handleSelected}

diff --git a/packages/ui-components/test/unit/components/UITreeDropdown.test.tsx b/packages/ui-components/test/unit/components/UITreeDropdown.test.tsx
@@ -187,6 +187,8 @@ describe('<UITreeDropdown />', () => {
             // In focuszone click callback handled also when ewnter key pressed on focused item
             wrapper.find('button.ms-ContextualMenu-link').first().simulate('click');
             expect(wrapper.state().value).toEqual('Title2');
+            expect(onChange).toBeCalledTimes(1);
+            expect(onChange).toBeCalledWith('Title2');
             // Try another select
             onChange.mockClear();
             openDropdown();
@@ -201,6 +203,8 @@ describe('<UITreeDropdown />', () => {
             // In focuszone click callback handled also when ewnter key pressed on focused item
             wrapper.find('button.ms-ContextualMenu-link').first().simulate('click');
             expect(wrapper.state().value).toEqual('Title2');
+            expect(onChange).toBeCalledTimes(1);
+            expect(onChange).toBeCalledWith('Title2');
             // Try another select
             onChange.mockClear();
             openDropdown();