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

[Select] The value matches with reference equality, document it #17764

Closed
LikiaSun opened this issue Oct 7, 2019 · 11 comments · Fixed by #17912
Closed

[Select] The value matches with reference equality, document it #17764

LikiaSun opened this issue Oct 7, 2019 · 11 comments · Fixed by #17912
Labels
component: select This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. hacktoberfest https://hacktoberfest.digitalocean.com/

Comments

@LikiaSun
Copy link

LikiaSun commented Oct 7, 2019

Select component's onChange is work, but it cannot render selected value.

import React, { useState } from 'react';
import InputLabel from '@material-ui/core/InputLabel';
import Select from '@material-ui/core/Select';
import MenuItem from '@material-ui/core/MenuItem';

export default function Test() {
  const [test, setTest] = useState({
    a: '',
    b: '',
    sum: {
      name: '',
    },
  });

  const option = [{ na: 'aa', nb: 'bb' }, { na: 'cc', nb: 'dd' }, { na: 'ee', nb: 'gg' }];

  const handleTestChange = e => {
    e.preventDefault();
    setTest({
      ...test,
      a: e.target.value.na,
      b: e.target.value.nb,
      sum: {
        ...test.sum,
        name: `${e.target.value.na} vs ${e.target.value.nb}`,
      },
    });
  };

  return (
    <div>
      <h1>test</h1>
      <h5>{test.sum.name}</h5>
      <InputLabel>Preview test</InputLabel>
      <Select value={test.sum.name} onChange={handleTestChange}>
        {option.map(option => (
          <MenuItem
            key={`${option.na + option.nb}`}
            value={{ na: option.na, nb: option.nb }}
          >{`${option.na} vs ${option.nb}`}</MenuItem>
        ))}
      </Select>
    </div>
  );
}

result like that
iss

also develop tool didn't pop any error or warning

@eps1lon
Copy link
Member

eps1lon commented Oct 7, 2019

@LikiaSun Please provide a full reproduction test case. This would help a lot 👷 .
A live example would be perfect. This codesandbox.io template may be a good starting point. Thank you!

@eps1lon eps1lon added the status: waiting for author Issue with insufficient information label Oct 7, 2019
@LikiaSun
Copy link
Author

LikiaSun commented Oct 7, 2019

hmm 🤔️ did i just write all of code on the top?
fine
LIVE SAMPLE: https://codesandbox.io/s/determined-satoshi-li3zb

@eps1lon eps1lon added component: select This is the name of the generic UI component, not the React module! and removed status: waiting for author Issue with insufficient information labels Oct 7, 2019
@eps1lon
Copy link
Member

eps1lon commented Oct 7, 2019

hmm thinking did i just write all of code on the top?

You did but provided no environment which we asked for. You should've been presented with an issue template that includes which values are relevant. A codesandbox is an alternative to a completely filled out issue template.

@eps1lon
Copy link
Member

eps1lon commented Oct 7, 2019

If you use objects in value you need to make sure these pass referential equality check. So if you have an array of values e.g. [{id: 1}, {id: 2}] you can't use <Select value={{id: 1}}><MenuItem value={{id: 1}} /></Select> but something like <Select value={values[selectedIndex]}><MenuItem value={values[0]} ></Select>.

We should include this caveat in the docs.

@eps1lon eps1lon added the docs Improvements or additions to the documentation label Oct 7, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 7, 2019

We should include this caveat in the docs.

@eps1lon Agree, it's a common problem people face. With some digging, we can find duplicated issues in the repository.

@oliviertassinari oliviertassinari changed the title [Select] Render Select value [Select] The value matches with reference equality, document it Oct 7, 2019
@LikiaSun
Copy link
Author

LikiaSun commented Oct 7, 2019

And also I just saw Select.renderValue api to let multple value or result value can be render to Select field.

https://codesandbox.io/s/serene-star-0bxs4

result can be render

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Oct 10, 2019
@prettyannoying
Copy link

Hi, is this issue resolved? Can I take it up, if not?

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 13, 2019

@skenypatel you are free to go :), regarding the solution, it seems that it's about mentionning the === equality check in the value prop description.

@oliviertassinari oliviertassinari added the hacktoberfest https://hacktoberfest.digitalocean.com/ label Oct 13, 2019
@oliviertassinari
Copy link
Member

@LikiaSun I think that we should wait for the documentation to be updated before closing the issue :).

@prettyannoying
Copy link

prettyannoying commented Oct 13, 2019

@skenypatel you are free to go :), regarding the solution, it seems that it's about mentionning the === equality check in the value prop description.

@oliviertassinari I'm new to this, I did not understand what exactly I am supposed to do. Can anyone guide me?

@adeelibr
Copy link
Contributor

@skenypatel You need to update the value prop documentation in filepackages\material-ui\src\Select\Select.js 😄

Particularly on Line no 190 as show below 😄

https://github.com/mui-org/material-ui/blob/0f28b62cc9d5b1c070f8959abb7eb9458e42c417/packages/material-ui/src/Select/Select.js#L190-L195

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: select This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. hacktoberfest https://hacktoberfest.digitalocean.com/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants