telerik / kendo-react

Issue tracker - KendoReact http://www.telerik.com/kendo-react-ui/
https://kendo-react-teal.vercel.app
Other
212 stars 39 forks source link

DropDownList fails to render text field with initial value #49

Closed Weffe closed 6 years ago

Weffe commented 6 years ago

I'm submitting a...

Current behavior

Currently, the DropDownList fails to render the textField correctly when initially setting the value of said DropDownList from componentDidMount. It does however correctly reflect the change of value from the componentDidMount.

Side note: The data is an array of objects that look like

{
    id: number;
    title: string;
    value: string;
}

Expected behavior

The DropDownList should render the textField properly when setting an initial value outside of the onChange function. This would be a controlled DropDownList.

Minimal reproduction of the problem with instructions

I have created a codesandbox demo perfectly recreating this bug. You can take a look at the console to see the current state. I have created the initial value to be the same as the first option in the dropdown list which is Animals.

Codesandbox: https://codesandbox.io/s/5wm4rp6orp Codesandbox Standalone: https://5wm4rp6orp.codesandbox.io/

Steps to take:

  1. Load demo
  2. Observe the current state after componentDidMount
  3. Select Animals
  4. Observe the new state
  5. textField correctly renders only after selecting the same value

What is the motivation or use case for changing the behavior?

The states between mounting and selecting the same option are exactly the same. However, what is not the same is the failure to render the textField even though the "different" states are exactly the same.

Environment

Package versions:

+-- @progress/kendo-data-query@1.3.1
+-- @progress/kendo-drawing@1.5.6
+-- @progress/kendo-react-buttons@0.5.1
+-- @progress/kendo-react-dateinputs@1.1.0
+-- @progress/kendo-react-dropdowns@1.1.0
+-- @progress/kendo-react-grid@1.1.0
+-- @progress/kendo-react-inputs@1.1.0
+-- @progress/kendo-react-intl@1.1.0
+-- @progress/kendo-react-layout@0.5.1
+-- @progress/kendo-react-pdf@1.1.0
+-- @progress/kendo-theme-bootstrap@2.13.5
+-- @types/axios-mock-adapter@1.10.0
+-- @types/classnames@2.2.4
+-- @types/jest@22.2.3
+-- @types/lodash.find@4.6.3
+-- @types/lodash.has@4.5.3
+-- @types/node@9.6.20
+-- @types/prop-types@15.5.3
+-- @types/react@16.3.16
+-- @types/react-dom@16.0.6
+-- @types/react-loadable@5.4.0
+-- @types/react-router-dom@4.2.7
+-- @types/react-test-renderer@16.0.1
+-- @types/reactstrap@5.0.27
+-- axios@0.18.0
+-- axios-mock-adapter@1.15.0
+-- UNMET PEER DEPENDENCY bootstrap@4.1.1
+-- classnames@2.2.6
+-- UNMET PEER DEPENDENCY cldr-data@^32.0.0
+-- core-js@2.5.7
+-- UNMET PEER DEPENDENCY jquery@1.9.1 - 3
+-- lodash.find@4.6.0
+-- lodash.has@4.5.2
+-- mobx@4.3.1
+-- mobx-react@5.2.3
+-- mocker-data-generator@2.6.4
+-- node-sass-chokidar@1.3.0
+-- npm-run-all@4.1.3
+-- prop-types@15.6.1
+-- react@16.4.0
+-- react-app-rewired@1.5.2
+-- react-dom@16.4.0
+-- react-loadable@5.4.0
+-- react-popper@0.10.4
+-- react-router-dom@4.2.2
+-- UNMET PEER DEPENDENCY react-scripts@^1.0.14
+-- react-scripts-ts@2.16.0
+-- react-test-renderer@16.4.0
+-- react-toastify@4.1.0
+-- react-transition-group@2.3.1
+-- reactstrap@5.0.0
+-- tslib@1.9.3
+-- typescript@2.9.1
`-- url-search-params-polyfill@3.0.0

Browser:

System:

Xizario commented 6 years ago

On a second look, it seems that the instances are different. Here is the modified working example: https://codesandbox.io/s/zl19j92pwm

Weffe commented 6 years ago

I took a look at the change you made and I'm wondering why does the value of the DropDownList need to come from the same array of data you are passing to the DropDownList?. Why can't it just have the same shape of data?

I made a change to get the DropDownList to render the text and have the correct value using the defaultItem prop that can be used on DropDownList. However, it ends up double rendering Animals inside the list. The major changes are on ln 110 and ln 47

https://codesandbox.io/s/jly3xzxzz5

Weffe commented 6 years ago

Also, @Xizario I recreated your working fix by using the provided filterBy function from the data-query package and I'm still getting the same results as in the original codesandbox.

Here's the filterBy sandbox: https://codesandbox.io/s/vmko587135

nstoychev commented 6 years ago

@Weffe - We are currently working on improving this behavior and hopefully for the next version of Kendo UI for React this workaround will not be needed.

About the defaultItem, its purpose is to indicate that nothing is selected in the DropDownList.

I have checked the example using filterBy and it looks like setting a function in the state twice does not work. Probably the second function overrides the first. Here is a modified example: https://codesandbox.io/s/506xmjy4q4

Weffe commented 6 years ago

Thanks, @nstoychev for the workaround. In my own app, I was using filterBy and would like to keep using it so I'll go with your solution to this issue. Hopefully, this does get fixed for the next version and will keep a lookout on it.

Xizario commented 6 years ago

@Weffe , my first example could have been a bit confusing in the whole context. Changing this:

<DropDownList
  data={categoryDropdownData}
  value={selectedCategoryDropdownValue}

to:

<DropDownList
  data={categoryDropdownData}
  value={categoryDropdownData.find(c => c.id === selectedCategoryDropdownValue.id)}

Is a bit more generic for use. This is not a workaround, it is the official way.

In future there will be a property that you can set to determinate field by which the item will be compared. But the solution above will still work. So for example it would be possible to do something like:

<DropDownList
  data={categoryDropdownData}  
  value={selectedCategoryDropdownValue}
  someFutureProperty="id"

That pretty much saves a few characters, nothing more.

In general we don't have plans to do deep equality check of the objects, since could cause lot of performance problems when your store and data become more complex. Shallow comparison on the other hand is not an option, because it could lead to false positive selects. Selecting by exact object reference is both safe and easy for customization.

nstoychev commented 6 years ago

We have published version 2.0.0 and the new property dataItemKey can also be used to resolve the issue.

Release notes can be found here: https://www.telerik.com/kendo-react-ui/components/changelogs/ui-for-react/.