improvement(treepicker): always using renderValue when value is exist (…

…#1139) * improvement(treepicker): always using renderValue when value is exist * fix(pickers): update renderValue logic and test case * fix(treepicker): remove unused var
rsuite · Jul 2, 2020 · 7603cea · 7603cea
1 parent 1e628ef
commit 7603cea
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 36 deletions.
diff --git a/src/CheckTreePicker/CheckTreePicker.tsx b/src/CheckTreePicker/CheckTreePicker.tsx
@@ -1165,23 +1165,35 @@ class CheckTreePicker extends React.Component<CheckTreePickerProps, CheckTreePic
       ...rest
     } = this.props;
     const { hasValue, selectedValues } = this.state;
-    const classes = getToggleWrapperClassName('check-tree', this.addPrefix, this.props, hasValue);
+    const hasValidValue = hasValue || (selectedValues.length > 0 && _.isFunction(renderValue));
+    const classes = getToggleWrapperClassName(
+      'check-tree',
+      this.addPrefix,
+      this.props,
+      hasValidValue
+    );
     const selectedItems = this.getSelectedItems(selectedValues);
     let selectedElement: React.ReactNode = placeholder;
 
-    if (hasValue && selectedValues.length) {
-      selectedElement = (
-        <SelectedElement
-          selectedItems={selectedItems}
-          countable={countable}
-          valueKey={valueKey}
-          labelKey={labelKey}
-          prefix={this.addPrefix}
-          cascade={cascade}
-          locale={locale}
-        />
-      );
-      if (renderValue) {
+    /**
+     * if value is invalid and renderValue is undefined, then using placeholder.
+     * if value is valid and renderValue is't undefined, then using renderValue()
+     */
+    if (selectedValues.length) {
+      if (hasValue) {
+        selectedElement = (
+          <SelectedElement
+            selectedItems={selectedItems}
+            countable={countable}
+            valueKey={valueKey}
+            labelKey={labelKey}
+            prefix={this.addPrefix}
+            cascade={cascade}
+            locale={locale}
+          />
+        );
+      }
+      if (_.isFunction(renderValue)) {
         selectedElement = renderValue(selectedValues, selectedItems, selectedElement);
       }
     }
@@ -1208,7 +1220,7 @@ class CheckTreePicker extends React.Component<CheckTreePickerProps, CheckTreePic
             onClean={createChainedFunction(this.handleClean, onClean)}
             componentClass={toggleComponentClass}
             cleanable={cleanable && !disabled}
-            hasValue={hasValue}
+            hasValue={hasValidValue}
             active={this.state.active}
           >
             {selectedElement || locale.placeholder}

diff --git a/src/CheckTreePicker/test/CheckTreePickerSpec.js b/src/CheckTreePicker/test/CheckTreePickerSpec.js
@@ -107,24 +107,48 @@ describe('CheckTreePicker', () => {
     assert.equal(instance.querySelector('.rs-picker-toggle-placeholder').innerText, 'test');
   });
 
-  it('Should render value by `renderValue`', () => {
+  it('Should output a value by renderValue()', () => {
+    const placeholder = 'value';
+
+    // Valid value
     const instance = getDOMNode(
       <CheckTreePicker
         data={[
-          { label: '1', value: '1' },
-          { label: '2', value: '2' }
+          { value: 1, label: '1' },
+          { value: 2, label: '2' }
         ]}
-        value={['1', '2']}
+        value={[1, 2]}
         renderValue={value => value.join(',')}
       />
     );
 
+    // Invalid value
+    const instance2 = getDOMNode(
+      <CheckTreePicker renderValue={v => [v, placeholder]} data={[]} value={[2]} />
+    );
+
+    // Invalid value
+    const instance3 = getDOMNode(
+      <CheckTreePicker
+        placeholder={placeholder}
+        renderValue={v => [v, placeholder]}
+        data={[]}
+        value={[]}
+      />
+    );
+
     assert.equal(instance.querySelector('.rs-picker-toggle-value').innerText, '1,2');
+    assert.equal(instance2.querySelector('.rs-picker-toggle-value').innerText, `2${placeholder}`);
+    assert.equal(instance3.querySelector('.rs-picker-toggle-placeholder').innerText, placeholder);
+  });
+
+  it('Should not be call renderValue()', () => {
+    const instance = getDOMNode(<CheckTreePicker data={[]} renderValue={() => 'value'} />);
+    assert.equal(instance.querySelector('.rs-picker-toggle-placeholder').innerText, 'Select');
   });
 
   it('Should render a placeholder when value error', () => {
     const instance = getDOMNode(<CheckTreePicker placeholder="test" data={data} value={['4']} />);
-
     assert.equal(instance.querySelector('.rs-picker-toggle-placeholder').innerText, 'test');
   });
 

diff --git a/src/TreePicker/TreePicker.tsx b/src/TreePicker/TreePicker.tsx
@@ -201,11 +201,18 @@ class TreePicker extends React.Component<TreePickerProps, TreePickerState> {
 
       this.focusNode(activeNode);
       this.unserializeLists('expand', expandItemValues);
+
+      let newState = {};
+      if (activeNode) {
+        newState = { activeNode: activeNode };
+      }
       this.setState({
-        data: nextData,
-        filterData,
-        activeNode,
-        expandItemValues: this.serializeList('expand')
+        ...{
+          data: nextData,
+          filterData,
+          expandItemValues: this.serializeList('expand')
+        },
+        ...newState
       });
     }
   }
@@ -1057,7 +1064,6 @@ class TreePicker extends React.Component<TreePickerProps, TreePickerState> {
       placeholder,
       cleanable,
       renderValue,
-      valueKey,
       labelKey,
       onEntered,
       onExited,
@@ -1066,15 +1072,25 @@ class TreePicker extends React.Component<TreePickerProps, TreePickerState> {
       positionRef,
       ...rest
     } = this.props;
-    const { activeNode } = this.state;
-    const classes = getToggleWrapperClassName('tree', this.addPrefix, this.props, !!activeNode);
+    const { selectedValue, activeNode } = this.state;
+    const hasValidValue =
+      !_.isNil(activeNode) || (!_.isNil(selectedValue) && _.isFunction(renderValue));
+    const classes = getToggleWrapperClassName('tree', this.addPrefix, this.props, hasValidValue);
 
     let selectedElement: React.ReactNode = placeholder;
     const hasValue = !!activeNode;
-    if (hasValue) {
-      selectedElement = activeNode?.[labelKey];
-      if (renderValue && activeNode) {
-        selectedElement = renderValue(activeNode[valueKey], activeNode, selectedElement);
+
+    /**
+     * if value is invalid and renderValue is undefined, then using placeholder.
+     * if value is valid and renderValue is't undefined, then using renderValue()
+     */
+    if (!_.isNil(selectedValue)) {
+      if (hasValue) {
+        selectedElement = activeNode[labelKey];
+      }
+      if (_.isFunction(renderValue)) {
+        const node = activeNode ?? {};
+        selectedElement = renderValue(selectedValue, node, selectedElement);
       }
     }
 
@@ -1101,7 +1117,7 @@ class TreePicker extends React.Component<TreePickerProps, TreePickerState> {
             onClean={createChainedFunction(this.handleClean, onClean)}
             cleanable={cleanable && !disabled}
             componentClass={toggleComponentClass}
-            hasValue={hasValue}
+            hasValue={hasValidValue}
             active={this.state.active}
           >
             {selectedElement || locale.placeholder}

diff --git a/src/TreePicker/test/TreePickerSpec.js b/src/TreePicker/test/TreePickerSpec.js
@@ -102,19 +102,51 @@ describe('TreePicker', () => {
     assert.equal(instance.querySelector('.rs-picker-toggle-placeholder').innerText, 'test');
   });
 
-  it('Should render value by `renderValue`', () => {
-    const instance = getDOMNode(
+  it('Should render value by `renderValue()`', () => {
+    const placeholder = 'value';
+
+    // valid value
+    const instance1 = getDOMNode(
       <TreePicker
         data={[
           { label: '1', value: '1' },
           { label: '2', value: '2' }
         ]}
         value={'2'}
-        renderValue={(value, item, selectedElement) => `Selected: ${item.label}`}
+        renderValue={(value, item) => `Selected: ${item.label}`}
+      />
+    );
+
+    // invalid value
+    const instance2 = getDOMNode(
+      <TreePicker
+        data={[
+          { label: '1', value: '1' },
+          { label: '2', value: '2' }
+        ]}
+        value={'5'}
+        renderValue={v => [v, placeholder]}
+      />
+    );
+
+    // invalid value
+    const instance3 = getDOMNode(
+      <TreePicker
+        placeholder={placeholder}
+        data={[]}
+        value={null}
+        renderValue={v => [v, placeholder]}
       />
     );
 
-    assert.equal(instance.querySelector('.rs-picker-toggle-value').innerText, 'Selected: 2');
+    assert.equal(instance1.querySelector('.rs-picker-toggle-value').innerText, 'Selected: 2');
+    assert.equal(instance2.querySelector('.rs-picker-toggle-value').innerText, `5${placeholder}`);
+    assert.equal(instance3.querySelector('.rs-picker-toggle-placeholder').innerText, placeholder);
+  });
+
+  it('Should not be call renderValue()', () => {
+    const instance = getDOMNode(<TreePicker data={[]} renderValue={() => 'value'} />);
+    assert.equal(instance.querySelector('.rs-picker-toggle-placeholder').innerText, 'Select');
   });
 
   it('Should render a placeholder when value error', () => {