[material-ui][joy-ui][base-ui][Autocomplete] Keep in sync highlighted index when the option still exists

[CGNonofr]

fix #41386

• I have followed (at least) the PR section of the contributing guide.

If the highlighted option still exists, it doesn't mean its index in the filtered list didn't change.

The index being out-of-sync with the visually highlighted item create weird behavior when interacting with the keyboard arrows (jumps)

Before: https://codesandbox.io/p/sandbox/mui-autocomplete-bug-94thpd
After: https://codesandbox.io/p/sandbox/mui-autocomplete-bug-forked-87nw9h

[mui-bot]

Netlify deploy preview

https://deploy-preview-41306--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 541c3f6

[ZeeshanTamboli]

@CGNonofr Are you trying to solve an existing GitHub issue? If not, could you start by creating a new issue and provide a CodeSandbox reproduction?

[CGNonofr]

@CGNonofr Are you trying to solve an existing GitHub issue? If not, could you please create a new issue with a CodeSandbox reproduction?

No, I didn't find any issue related to it

I sure can create an issue if you process needs it.

Regarding the CodeSandbox reproduction, I've already changed a test (the last commit) that highlight the issue, is that enough?

[ZeeshanTamboli]

I sure can create an issue if you process needs it.

Regarding the CodeSandbox reproduction, I've already changed a test (the last commit) that highlight the issue, is that enough

A codesandbox reproduction would still help.

[CGNonofr]

I sure can create an issue if you process needs it.
Regarding the CodeSandbox reproduction, I've already changed a test (the last commit) that highlight the issue, is that enough

A codesandbox reproduction would still help.

Created the issue and linked it in the PR description (#41386)

[ZeeshanTamboli]

The fix doesn't work as expected. I tested it using this PR's build: "@mui/material": "https://pkg.csb.dev/mui/material-ui/commit/920b7e5f/@mui/material". See the CodeSandbox - https://codesandbox.io/p/sandbox/mui-autocomplete-bug-forked-tg875l.

[CGNonofr]

The fix doesn't work as expected. I tested it using this PR's build: "@mui/material": "https://pkg.csb.dev/mui/material-ui/commit/920b7e5f/@mui/material". See the CodeSandbox - https://codesandbox.io/p/sandbox/mui-autocomplete-bug-forked-tg875l.

for it to work, we also need to add isOptionEqualToValue={(a, b) => a.id === b.id}

[ZeeshanTamboli]

for it to work, we also need to add isOptionEqualToValue={(a, b) => a.id === b.id}

Why is it necessary? The prop is optional. Keyboard navigation works in general (no option insertion) without it being required. What's the rationale behind needing it in this specific case?

[CGNonofr]

for it to work, we also need to add isOptionEqualToValue={(a, b) => a.id === b.id}

Why is it necessary? The prop is optional. Keyboard navigation works in general (no option insertion) without it being required. What's the rationale behind needing it in this specific case?

because all the items are changed, the alternative is to reuse the items: https://codesandbox.io/p/sandbox/mui-autocomplete-bug-forked-mxsvf7?file=%2Fsrc%2FApp.js%3A22%2C68

[ZeeshanTamboli]

for it to work, we also need to add isOptionEqualToValue={(a, b) => a.id === b.id}

Why is it necessary? The prop is optional. Keyboard navigation works in general (no option insertion) without it being required. What's the rationale behind needing it in this specific case?

because all the items are changed, the alternative is to reuse the items: https://codesandbox.io/p/sandbox/mui-autocomplete-bug-forked-mxsvf7?file=%2Fsrc%2FApp.js%3A22%2C68

How would developers know they need to use that prop? Also, the above CodeSandbox is inaccessible. Perhaps you need to grant permissions to me.

[CGNonofr]

for it to work, we also need to add isOptionEqualToValue={(a, b) => a.id === b.id}

Why is it necessary? The prop is optional. Keyboard navigation works in general (no option insertion) without it being required. What's the rationale behind needing it in this specific case?

because all the items are changed, the alternative is to reuse the items: https://codesandbox.io/p/sandbox/mui-autocomplete-bug-forked-mxsvf7?file=%2Fsrc%2FApp.js%3A22%2C68

How would developers know they need to use that prop?

You're right, the code is not correct, I will come up with something better

It still feel weird to me that the option comparison rely exclusively on the label without any way to override that behavior (so it's impossible to rename an option without losing it's highlighting)

Also, the above CodeSandbox is inaccessible. Perhaps you need to grant permissions to me.

my bad, it's now shared

[ZeeshanTamboli]

It still feel weird to me that the option comparison rely exclusively on the label without any way to override that behavior (so it's impossible to rename an option without losing it's highlighting)

I didn't understand what you mean. You can use getOptionLabel to rename an option label.

[CGNonofr]

@ZeeshanTamboli ok I've rewritten the PR

[ZeeshanTamboli]

Thanks for this fix. The logic and the fix seem solid, but the tests are passing on the master HEAD branch which indicate they're not covering the necessary scenario. Maybe add a new test instead of modifying an existing one. Also, need to update the comment.

[CGNonofr]

Thanks for this fix. The logic and the fix seem solid, but the tests are passing on the master HEAD branch which indicate they're not covering the necessary scenario. Maybe add a new test instead of modifying an existing one. Also, need to update the comment.

Rewritten the PR once again!

[ZeeshanTamboli]

Looks good now! Thanks for the fix!