Skip to content

Pressing ENTER should propagate when a closed Select has focus#2539

Closed
MrLeebo wants to merge 1 commit intoJedWatson:masterfrom
MrLeebo:submit-on-enter
Closed

Pressing ENTER should propagate when a closed Select has focus#2539
MrLeebo wants to merge 1 commit intoJedWatson:masterfrom
MrLeebo:submit-on-enter

Conversation

@MrLeebo
Copy link
Contributor

@MrLeebo MrLeebo commented Apr 20, 2018

I'm opening this PR to start a conversation rather than to get what's here merged necessarily. There appears to be competing motivations for better accessibility support vs native <select> compatibility. I think the pendulum has swung too far away from native compatibility with a recent change. In v1.2.1 if I have this:

<form onSubmit={props.handleSubmit}>
  <Select />
</form>

I should be able to press ENTER when the Select menu is closed to submit the form. This is how it worked in v1.0.0, but #1428 changed the behavior to open the menu instead. I don't know what an appropriate compromise should be for accessibility, but not allowing ENTER to propagate runs counter to a lot of people's expectations, I think. It seems that this should be fine, because they can press virtually any other key on the keyboard to open the menu, but I'm not an expert so I was hoping to discuss.

Related issues: #2217 and #518

I created a build branch with these changes applied so if you want to try out these changes in an app before this PR gets merged, you can change your react-select dependency to:

"react-select": "git://github.com/MrLeebo/react-select.git#build",

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 94.837% when pulling 7cf2813 on MrLeebo:submit-on-enter into 404bf14 on JedWatson:master.

@degr
Copy link

degr commented May 11, 2018

@MrLeebo, Thanks for decision. Unfortunately I need this future yesterday, but PR still in open status.

So, currently I found this way how to override default behavior:

import Select from "react-select";

export default class InternalSelect extends Select {
    handleKeyDown(event) {
        //todo remove this class when https://github.com/JedWatson/react-select/pull/2539 will be merged
        if (this.props.disabled) return;
        if (typeof this.props.onInputKeyDown === 'function') {
            this.props.onInputKeyDown(event);
            if (event.defaultPrevented) {
                return;
            }
        }
        if(event.keyCode === 13) {
            if (!this.state.isOpen) {
                return;
            }
            event.preventDefault();
            event.stopPropagation();
            this.selectFocusedOption();
        } else {
            super.handleKeyDown(event);
        }
    }
}

@gwyneplaine
Copy link
Collaborator

@MrLeebo, thanks for this. This is behaviour that has been removed from v2, specifically for cases like this. In react-select v2, enter no longer opens the menu or stop event propagation.

We won't be making this change in v1, as it is a major breaking change to functionality

@gwyneplaine gwyneplaine closed this Jul 4, 2018
@degr
Copy link

degr commented Oct 24, 2018

Issue not resolved in V2, it event become worse.
If in V1 I can inherit Select and override onKeyDown function, in V2 it's impossible, because library exports manageState(Select), which use Select as composition element, so, inheritance is impossible.
Now, when I click enter button, this lines of code will be executed:

switch (event.key) {
   ....code
    case 'Enter':
        if (menuIsOpen) {
          if (!focusedOption) return;
          if (isComposing) return;
          _this7.selectOption(focusedOption);
        } else {
          _this7.focusOption('first');
        }
        break;
   ...code
}
event.preventDefault();

and as you can see, this handler will _this7.focusOption('first'); and then event.preventDefault();,
I need this event without preventDefault. Is there any ways how to do this?

@degr
Copy link

degr commented Oct 24, 2018

I solve issue. To do it, I pass in all my selects "formSubmit" handler, I start listen onMenuOpen and onMenuClose events, and inside of onKeyDown handler I was able to determinate that select have closed drop-down list, that enter key was pressed and call formSubmit instead of html submit event. Realy guys, in V1 it was more simple.

@degr
Copy link

degr commented Oct 31, 2018

Actually, I found if you will throw and exception inside of your onKeyDown function, event will not be prevented, and, most probably, it will bubble to your form. Yes, throw exception.

@jitenderchand1
Copy link

Any update on this? This supposed to be default behaviour

@TrySound
Copy link
Contributor

TrySound commented Feb 5, 2019

@gwyneplaine This shit is broken. We have react-select in 50 forms and they are not accessible.

@gwyneplaine gwyneplaine reopened this Feb 6, 2019
@gwyneplaine
Copy link
Collaborator

@TrySound noted, PR for it incoming;

@gwyneplaine gwyneplaine closed this Feb 7, 2019
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.

6 participants