Prevent ArrowLeft & ArrowRight key action when controlShouldRenderValue=false

[kidokidozh]

Problem We are using the select component with controlShouldRenderValue = false. And we have defined event handler for ArrowLeft & ArrowRight key on the container element. But the event handler here does not return, and go to event.preventDefault(), which caused our event handler not work.

Solution Just return when controlShouldRenderValue = false, since it does not make sense to focusValue when no value rendered.

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: c77ca73fd1ce829430c5704526890e4bf7696c55

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c77ca73fd1ce829430c5704526890e4bf7696c55:

Sandbox	Source
react-codesandboxer-example	Configuration

[kidokidozh]

@JedWatson Could you review this? Thanks

[Rall3n]

Hello @kidokidozh,

thank you for your contribution. We are currently in the middle of our TypeScript/v5 beta release. Therefore we are currently not merging any PR until this is finished.

But I had a quick look at your proposed changes and I think that these changes are not necessary. I don't know how you applied your event handler, but the Select component provides a prop named onKeyDown with which you can overwrite the handling of certain or all key events which are handled by the component.

<Select
    onKeyDown={e => {
        // Skip if not keys we want
        if (!['ArrowLeft', 'ArrowRight'].includes(e.key)) return;
        
        // Need to prevent or default handling runs after this.
        e.preventDefault();
        
        // Do your code
    }}
/>

Based on what you wrote in your first comment, this should be sufficient. If not, then please explain.

[kidokidozh]

Hi @JedWatson , thanks for your reply, sorry I didn't explain the problem clearly.

We are using Select inside a table like this.

For each row, the expect behavior is pressing ArrowLeft/ArrowRight will move the cursor to next column's input, e.g. from "Name" input to "Type" select. So we add an event handler on the row element.

<tr onKeyDown={e => {
    if (['ArrowLeft', 'ArrowRight'].includes(e.key)) { //focus to next column's input }
}}>
    <td><input /></td>
    <td><Select /></td>
    <td><Select /></td>
</tr>

The problem is Select will catch the Arrow key event, and prevent default in this line. This causes our row event handler not working. https://github.com/JedWatson/react-select/blob/master/packages/react-select/src/Select.tsx#L1489

My understanding is that, since the ArrowLeft/ArrowRight event defined here is used to focus to previous/next value under multi value mode, it's reasonable to add a return condition when controlShouldRenderValue=false. Because when it's false the default multi value component is not rendered, it makes no sense to focus previous or next value. https://github.com/JedWatson/react-select/blob/master/packages/react-select/src/Select.tsx#L1385

case 'ArrowLeft':
    if (!isMulti || inputValue || !controlShouldRenderValue) return;
    this.focusValue('previous');
    break;

[ebonow]

I think there is some merit to this PR. I don't think that it makes sense to add keybindings to the left and right arrows if the user has chosen not to render the values in the control.

This example shows that the user wants the event to continue propagation to the row event handler.

To @Rall3n's point though, this is not required for this functionality to work. Here is a codesandbox I was able to implement using arrow keys to switch focus and regardless of the props passed into the Select, the focus is still passed to the next input/control.

https://codesandbox.io/s/react-select-event-propagation-next-focus-arrow-key-left-right-ysf51?file=/src/App.js

[bbsimonbb]

preventDefault and stopping propagation are very aggressive manoeuvres. In my code, I add a history string array to all events, Then any function that handles an event leaves its trace, and downstream handlers can examine what action has already been taken on the event. It's such a good idea, I'm happy to share it.

export interface IHasHistory {
    history: string[];
}

export function withHistory<TargetShape>(target: any): TargetShape & IHasHistory {
    if (!target.history) {
        (target as IHasHistory).history = [];
    }
    return target;
}

then, any time you call a handler function...

onclick={(e)=>myHandler(withHistory(e))}

and

function myHandler(e:ClickEvent & IHasHistory){
   if(e.history.length === 0){ // or whatever test you like
      ...
      e.history.push("myHandler")
   }
}

[bbsimonbb]

@ebonow how can I modify your sandbox such that the right arrow key saves the currently focussed option in the react-select please ?