Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Joy][Select] onChange callback fires without user interaction #36783

Closed
2 tasks done
sibljon opened this issue Apr 5, 2023 · 7 comments · Fixed by #37141
Closed
2 tasks done

[Joy][Select] onChange callback fires without user interaction #36783

sibljon opened this issue Apr 5, 2023 · 7 comments · Fixed by #37141
Assignees
Labels
bug 🐛 Something doesn't work component: select This is the name of the generic UI component, not the React module! package: base-ui Specific to @mui/base package: joy-ui Specific to @mui/joy

Comments

@sibljon
Copy link

sibljon commented Apr 5, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Link to live example:

https://codesandbox.io/s/select-onchange-callback-mb1n7d?file=/demo.tsx

Steps:

  1. Use a Select component
  2. Have its initial value be anything (for example: null) by specifying the value in the value prop
  3. Change its initial value via the value prop
  4. !! Observe that the onChange handler is called

Current behavior 😯

The onChange callback fires even though the value did not change due to user interaction

Expected behavior 🤔

The onChange callback only fires when the value changes due to user interaction

Context 🔦

It's a common pattern to render a Select and not have its value until later, such as when a network request completes. The owner of the Select component is both specifying the value and the onChange handler, and thus, it doesn't seem necessary that the owner would need to know when the very value that it's supplying has changed.

I've used other UI libraries such as Semantic-UI and UIKit on iOS, and change handlers for UI components are always only called when a value changes due to user interaction. I was hoping that someone here could help me understand why the onChange callback is fired in this case. I first thought this was a bug, but given that there are seemingly no Github Issues related to it, I'm wondering if it's intentional. I noticed that the event is null when the change is not due to user-interaction, so that can at least be used in the short-term to ignore these onChange events when they are not due to user interaction.

Your environment 🌎

npx @mui/envinfo

  System:
    OS: macOS 13.2.1
  Binaries:
    Node: 14.19.1 - ~/.nvm/versions/node/v14.19.1/bin/node
    Yarn: 3.4.1 - /opt/homebrew/bin/yarn
    npm: 6.14.16 - ~/.nvm/versions/node/v14.19.1/bin/npm
  Browsers:
    Chrome: 112.0.5615.49
    Edge: Not Found
    Firefox: 111.0.1 (I'm using Firefox currently)
    Safari: 16.3

@sibljon sibljon added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 5, 2023
@zannager zannager added component: select This is the name of the generic UI component, not the React module! design: joy This is about Joy Design, please involve a visual or UX designer in the process labels Apr 6, 2023
@siriwatknp
Copy link
Member

siriwatknp commented Apr 14, 2023

I agree with @sibljon so mark this as a bug. I think this might related to Base UI than Joy UI. @michaldudak Would the latest refactor of the Listbox fixes this?

@siriwatknp siriwatknp added package: base-ui Specific to @mui/base package: joy-ui Specific to @mui/joy bug 🐛 Something doesn't work and removed design: joy This is about Joy Design, please involve a visual or UX designer in the process status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Apr 14, 2023
@michaldudak
Copy link
Member

Yes, this issue should be fixed on master (it will be released on Monday or Tuesday).

@homerchen19
Copy link
Contributor

homerchen19 commented Apr 26, 2023

@michaldudak I'm currently using the latest version (5.0.0-alpha.127) of @mui/base, but I've still come across a similar issue.

onChange callback appears to fire a null value without any user interaction, if the initial value doesn't match any of the available option values. I'm wondering if this behavior is intentional?

Code snippet:

export default function UnstyledSelectSimple() {
  const [value, setValue] = React.useState<string>("");

  return (
    <CustomSelect<string, false>
      value={value}
      onChange={(_, value) => {
        console.log(value); // null
        setValue(value);
      }}
    >
      <StyledOption value="10">Ten</StyledOption>
      <StyledOption value="20">Twenty</StyledOption>
      <StyledOption value="30">Thirty</StyledOption>
    </CustomSelect>
  );
}

Here is a codesandbox example: https://codesandbox.io/s/nifty-kare-miz8r6?file=/demo.tsx

Screenshot 2023-04-26 at 17 34 49

@michaldudak
Copy link
Member

Right, the issue was still present when you set the initial value to an empty one. I just submitted a PR to fix this.

However, the codesandbox that you linked to has another issue - you're setting the value to "", but it's not a valid option. In Base UI's Select you have to pass in null if you don't want any options to be selected. An empty string is treated like any other value (and since it doesn't exist among options, the select resets itself to null calling the onChange handler).

@martsim6
Copy link

Hello, I have same/similar issue using BaseUI Select with multiple option.
When passing some initial values through form (formik initial values), Select takes those values first, but even they are same objects, it fires the onChange event with empty array as new value and resets initial value. Looks like some deep compare is missing in its logic.

little sandbox without form but state init value: https://codesandbox.io/s/hopeful-visvesvaraya-r527yc?file=/demo.tsx

Do you have any workaround till it's fixed please?

I thought about skipping onChange when newValue is empty array, but it's not a good idea, cuz user can also trigger onChange with empty array (when clearing select or taking away last option selected), so it's not possible do it that way.

@michaldudak
Copy link
Member

@martsim6 open a new issue, please. I'll look into it.

@Kmg11
Copy link

Kmg11 commented Jan 6, 2024

A simple fix just check if e not null
onChange=((e) => {
if (e) {
// Your code
}
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: select This is the name of the generic UI component, not the React module! package: base-ui Specific to @mui/base package: joy-ui Specific to @mui/joy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants