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

fix(ui-simple-select): fix selection getting lost after options have changed #1821

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

matyasf
Copy link
Collaborator

@matyasf matyasf commented Dec 11, 2024

Closes: INSTUI-4402

This bug was introduced by #1674

This issue came up when the Option's value and children differed.

TEST PLAN:
make a SimpleSelect where the Option's value and children are not the same. Select a child and remove a different element from the child list. Selection should stay. Test with this code:

const InnerComponent = ({ handleSelect, options, value }) => {
// should work here SimpleSelect.Option's value is opt.name too
  return (
    <SimpleSelect
      renderLabel="Controlled Select"
      value={value.toString()}
      onChange={handleSelect}
    >
      {options.map((opt) => (
        <SimpleSelect.Option
          key={opt.id}
          id={opt.id.toString()}
          value={opt.id.toString()}
        >
          {opt.name}
        </SimpleSelect.Option>
      ))}
    </SimpleSelect>
  );
};

const allOptions = [
  { id: 1, name: "Catalog 1" },
  { id: 2, name: "Catalog 2" },
];

const Example = () => {
  const [value, setValue] = useState(allOptions[0].id);
  const [skipSecond, setSkipSecond] = useState(false);

  const handleSelect = (e, { value }) => setValue(value);

  const options = allOptions.filter(
    (opt) => opt.id !== allOptions[1].id || !skipSecond
  );

  console.log(value, options);

  return (
    <>
      <InnerComponent
        handleSelect={handleSelect}
        options={options}
        value={value}
      />
      <Checkbox
        checked={skipSecond}
        label="Skip Second"
        onChange={() => setSkipSecond(!skipSecond)}
      />
    </>
  );
};

render(<Example />);

…changed

Closes: INSTUI-4402

This issue came up when the Option's value and children differed.
TEST PLAN:
make a SimpleSelect where the Option's value and children are not the same. Select a child and
remove a different element from the child list. Selection should stay
@matyasf matyasf requested review from balzss and ToMESSKa December 11, 2024 09:04
@matyasf matyasf self-assigned this Dec 11, 2024
Copy link

github-actions bot commented Dec 11, 2024

PR Preview Action v1.4.8
Preview removed because the pull request was closed.
2024-12-13 17:19 UTC

Copy link
Contributor

@balzss balzss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works well, nice job

@matyasf matyasf requested a review from joyenjoyer December 13, 2024 12:39
Copy link
Contributor

@joyenjoyer joyenjoyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice one

@matyasf matyasf merged commit 4e07f9a into master Dec 13, 2024
11 checks passed
@matyasf matyasf deleted the fix_select_data_change branch December 13, 2024 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants