[ComboBox] Change tabindex prop to a negative number

[adamcarchibald]

WHY are these changes introduced?

Resolves #1089

Fixes tab navigation behaviour in the Autocomplete component by not selecting the ComboBox when tabbing through the component.

WHAT is this pull request doing?

Changes the tabIndex prop value from 0 to -1 for the ComboBox component. Adds a regression test.

[ghost]

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines. You can also join #polaris on the Shopify Partners Slack.

[adamcarchibald]

I have removed the regression test as I am unable to determine a pattern for testing the tab navigation behaviour. I was following the pattern set out in similar tests for ComboBox but we discovered they are false positives as per #1128

[dpersing]

I'm wondering if a tabindex attribute is needed at all? The container won't take focus naturally, so I think it's safe to remove it altogether (unless there's something hooked into that specifically, in which case I'm wondering why that might be!).

[AndrewMusgrave]

Were you able to tell if it's required for some reason?

[ry5n]

I chatted with @adamcarchibald this morning. Rather than have this stall, the simplest thing might be to add a comment to ignore the lint rule to get this unblocked.

(In that case, The Polaris team should circle back later to fix the combobox implementation.)

[danrosenthal]

What's the next step here, @adamcarchibald? Should this be closed?

[adamcarchibald]

It was suggested that I simply remove the linting rule that is blocking this pr. Will make changes now and post update.

[danrosenthal]

I spent some time this morning tophatting this change, and it has negative impact on the focus and keyboard behavior when compared to its current state in master. Both are unusable when interacted with using the arrow keys to move through available items.

We've documented the extensive problems with this component here: #689. Rather than spending time trying to patch something with deep issues, we should focus on rebuilding.

I think it makes sense to close this pull request and for the Polaris team to prioritize fixing this component.