focus start from selected option

[ankush29]

No description provided.

[changeset-bot]

💥 No Changeset

Latest commit: a67c548

Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂

If these changes should be published to npm, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[jamieshark]

Hey @JedWatson + other maintainers - 👋🏼 I'm on the front end platform team for Trello, and we're currently using react-select to upgrade our current <select>s to match up closely with AK Selects. This PR fixes a bug that is currently preventing us from rolling this change out publicly. I've tried a few workarounds using ref props, but it seems like the way that the focusedOption element is getting set is a little too deep in the source code to reasonably reach from a higher level. If there's anything we can do to encourage getting this merged OR if there's a workaround that I might be currently missing, please let me know. 🙂

[JedWatson]

Hey @ankush29 thanks for the PR and @jamieshark thanks for the offer to help get it over the line.

This one (and #3815 and #3814) are pretty hard to review without any context on

• the details of the bug they're fixing
• how to reproduce the bug
• why this particular fix solves the bug, and
• what other effects could arise from the change

In this case, it looks like this change could introduce issues because not all options are expected to be focusable, which is why the menuOptions.focusable array is being used and not the options prop. It also looks like it will prevent focus being given to the "next" option, always selecting the first, which is a behaviour change.

Can you help answer the points above? Thanks!

[jamieshark]

@JedWatson - thanks for responding!

Here's some more context from our end:
Bug details
The "bug" we're dealing with is that if you have a value or defaultValue for the Select, when you open the menu, the value that you have selected does not get scrolled into view. With native selects (not saying that the behavior has to be 1:1, just what we're comparing to), when you open the select, the selected value is already "in view."
How to reproduce

1. Choose any option on the Select that is not within initial view of the menu options (you must scroll to select the value)
2. Close select menu
3. Re-open select menu
4. Confirm that the selected value is not automatically scrolled to within menu options

You bring up a good point of not all menu options being focusable. I also tried to create a workaround for this by using the focusedOption ref, but it doesn't seem to be getting assigned as expected (in that it is not returning a value at all). Any other thoughts?

[Tirzono]

An example on CodeSandbox on how the focus is not working properly:

https://codesandbox.io/s/autumn-wind-l3ed8?fontsize=14

Note that whatever the selected option is, when opening the menu it will always focus the first option.

[jabsatz]

Hello! I'm closely following this because it'd be useful for the app I'm working on as well. Luckily I found a workaround, it flickers a little at the beggining, but it's good enough until this is merged:

<Select
  ref={ref}
  options={options}
  value={value}
  onMenuOpen={() => {
    setTimeout(() => {
      ref.current.select.scrollToFocusedOptionOnUpdate = true;
      ref.current.select.setState({
        focusedOption: options.find(option => option.value === value)
      });
    }, 0);
  }}
/>

[olu1987]

Hey guys, is this getting merged any time soon? @JedWatson

[Tirzono]

Since it looks like there is no progress on this PR, I've opened a new one in #3868.

[Avery246813579]

@JedWatson is there any update on when you could look at this PR. This is a blocking bug.

[Tirzono]

@Avery246813579 it looks like this PR won't be merged because it contains some errors (as pointed out by @JedWatson) and since then there has not been any progress on this PR. There's a new pull request #3868 that contains a fix while respecting @JedWatson's comments. So far the maintainers didn't have any time to have a look at it, so we just patiently have to wait until they do have time to provide feedback or merge the changes.

[olu1987]

I ended up rolling back to v3.0.3 in order to fix this issue. Some of you guys may have to do the same until this issue is resolved properly.

[Huanghuiying0624]

Is there any update for this issue?

[Avery246813579]

@Huanghuiying0624 nope, seems like the only way to get this working is rolling back to 3.0.3.

[flexdinesh]

PR #3868 addresses this issue. Fix released in version 3.1.0. Thanks y'all! Closing this now.