palantir / blueprint

A React-based UI toolkit for the web
https://blueprintjs.com/
Apache License 2.0
20.67k stars 2.17k forks source link

[select] Select component doesn't respect the query prop #4362

Open cemreyavuz opened 4 years ago

cemreyavuz commented 4 years ago

Environment

Code Sandbox

https://codesandbox.io/s/select-component-doesnt-respect-query-prop-dlih7

Steps to reproduce

  1. Import the Select component from @blueprintjs/select and render
  2. Pass the query prop to a Select component
  3. Click the rendered Select component and update the text input

Actual behavior

Text input is updated.

Expected behavior

As mentioned in the documentation of Select component, the expected behavior is that the text input is not changed and controlled via query prop.

inputProps Props to spread to the query InputGroup. Use query and onQueryChange instead of inputProps.value and inputProps.onChange to control this input.

Possible solution

Updating line 134 in select.tsx from:

value={listProps.query}

to this:

value={this.props.query || listProps.query}

solves the problem. It seems like the query prop is never directly passed to the input group. QueryList component is the one who controls the query of InputGroup in Select component. It initializes its internal query state with the query prop, but from that point onward, it doesn't respect the query prop of Select component and send its internal state to InputGroup.

adidahiya commented 3 years ago

What exactly is the problem in your code sandbox? It seems like the query prop control is working as intended.

I briefly traced through the rendering stack of Select / QueryList and it looks like listProps.query is being set to QueryList's state.query (which is kept up to date via QueryList's componentDidUpdate) here:

https://github.com/palantir/blueprint/blob/fd3833e0ced0befbd5819e1690404cfd7705a084/packages/select/src/components/query-list/queryList.tsx#L206

... which means we achieve the same effect as referencing this.props.query as you've suggested.

are you able to produce a failing unit test you could perhaps work with?

nickensoul commented 3 years ago

https://user-images.githubusercontent.com/14836054/122240307-5897ef80-ceca-11eb-9b59-c32543fa4464.mov

@adidahiya in the screencast attached, there should not be any other values in the input other than "constant query" after changing event

adidahiya commented 2 years ago

@nickensoul does #4363 fix the issue from your screen recording?