react icon indicating copy to clipboard operation
react copied to clipboard

FeatureRequest: SelectPanel without save and cancel buttons

Open deleonjuan opened this issue 2 years ago • 4 comments

Describe the bug Recently the SelectPanel component was updated to add cancel and save buttons at the bottom of the component, this is great but in some cases i think it's innecesary (for example when you can only select 1 option instead of an array, due the component allows to do that) so... a prop that allows hide or show the buttons can be usefull or even maybe a bottom prop, so the dev can choose what he wants to show there

Screenshots image image image

deleonjuan avatar Aug 27 '22 16:08 deleonjuan

Thanks for writing this up!

@alliethu could you take a look at this request from the accessibility design perspective and let us know if this pattern without the save and cancel buttons would still be accessible?

lesliecdubs avatar Sep 12 '22 23:09 lesliecdubs

👋 @deleonjuan I recorded a call with @jscholes, one of our Screen reader expert consultants, to explore your proposal.

📹 Our meeting recording

User scenario: A user activates a selectPanel, clicks/presses to select an option, the option is automatically saved, and the dialog closes.

As @jscholes mentions here, although this may seem like an accessible solution to mouse users, this is an inaccessible pattern for keyboard users. Reason being, there is no cue for keyboard users to know that pressing enter selects and saves the option, thus closing the dialog. Keyboard users need the opportunity to arrow through the options, select an option (pressing enter), and then be able to tab to save/cancel the options.

alliethu avatar Sep 14 '22 16:09 alliethu

@alliethu Just to clarify: keyboard users don't need to press Enter to select an option, and then Tab to "Save". Having Enter choose an item and close the select panel is fine, it's just non-obvious to users who don't know they can do so. So for those users, they must be able to arrow to an option, and discover the "Save" button.

jscholes avatar Sep 14 '22 17:09 jscholes

Thank you for that clarification, @jscholes!

alliethu avatar Sep 14 '22 17:09 alliethu

Thanks for the accessibility review!

Given that this proposal would lead us to implementing an inaccessible design pattern, it's not something we'll be moving forward with. Please feel free to comment back, even once this issue is closed, if there are further questions or concerns.

lesliecdubs avatar Sep 23 '22 01:09 lesliecdubs

I'd like to challenge this and ask if we can find a solution to this without the Save button. Are there some cues we can give users that pressing Enter would select and Close?

I'm all for adding the Save button if it leads to a more accessible experience, but one of our main customer complaints is that it requires a lot of clicks to do something.

A competitor of ours, Linear, uses a pattern that I feel works quite well: Pressing Enter in a select panel selects and closes it. Pressing Space selects but keeps it open.

Another difference in their implementation is that changes are applied instantly (as in the user doesn't have to close or save the panel to see changes take effect) - I'm wondering if that's an inaccessible solution or if they're doing something that makes this a great experience for all users.

six7 avatar Sep 23 '22 07:09 six7

@primer/accessibility-design

alliethu avatar Sep 26 '22 15:09 alliethu

@six7 I'd like to try and clear up some points of confusion:

I'm all for adding the Save button if it leads to a more accessible experience, but one of our main customer complaints is that it requires a lot of clicks to do something.

These clicks should not be required. In my accessibility sign off review for this component (https://github.com/github/primer/issues/1245#issuecomment-1242376489), I indicated that the behaviour for single-select use cases is currently suboptimal, and that Enter/click should be available to choose an item and close the panel. That is not currently implemented within the component, but hopefully will be in the near future.

At that point, using the "Save" button will not be strictly required for single-select, for any user. However, it's important that it still be present from a usability and consistency perspective, so that users unfamiliar with the "Enter to select" pattern can still work out what to do. It also keeps the component UI balanced, in terms of having a "Cancel" button too, and not only relying on the "Close" button at the top of the dialog for dismissal.

For multi-select, the "Save" button is required at all times, although pressing Enter on an option can close the panel without toggling that option. It would not be accessible to have a multi-select pattern in place without explicit save/cancel buttons.

Another difference in their implementation is that changes are applied instantly (as in the user doesn't have to close or save the panel to see changes take effect) - I'm wondering if that's an inaccessible solution

It is not. This is universally bad for accessibility across the board, as it limits exploration, and can leave users in an unintended state without having the means to undo it. See 3.2.2 On Input (level A) in WCAG for more context.

jscholes avatar Sep 26 '22 17:09 jscholes

I don't have anything else to add here other than I 100% agree with @jscholes!

ichelsea avatar Sep 29 '22 18:09 ichelsea

Can we do something to mark https://github.com/primer/react/issues/2342 as blocked until the above changes are implemented and this has had a chance for user feedback?

As @six7 has covered this would have some major customer pushback for us if it were to be released as is. This essentially would block us from any primer upgrades if it become part of a major version release as seems to be indicated here - https://github.com/primer/react/issues/2342#issuecomment-1251650056

Where is is not clear if the plan is to put it in draft because it is not ready to be consumed yet (until the feedback from @jscholes is implemented) or to put it in draft for minor version and then release fully in the next major version.

azenMatt avatar Sep 30 '22 09:09 azenMatt

@azenMatt:

Can we do something to mark https://github.com/primer/react/issues/2342 as blocked until the above changes are implemented and this has had a chance for user feedback?

Noting here that #2342 has been cross-linked to this issue.

Where is is not clear if the plan is to put it in draft because it is not ready to be consumed yet (until the feedback from @jscholes is implemented) or to put it in draft for minor version and then release fully in the next major version.

The feedback @jscholes referenced above https://github.com/github/primer/issues/1245#issuecomment-1242376489 was actually already implemented and released in a recent Primer React minor version, but in doing so we broke the semver expectations for our consumers because the work introduced a breaking change. I'll work with the team to get an update on when the next major version is likely to go out https://github.com/github/primer/issues/1294#issuecomment-1278477990.

lesliecdubs avatar Oct 14 '22 04:10 lesliecdubs

After discussing with PRC maintainers, we've determined that releasing the next major before Universe risks bugs, churn, and frustration from consumers. As per https://github.com/github/primer/issues/1294#issuecomment-1289723555, we're currently planning to release the version with SelectPanel fixes in January 2023. FYI @azenMatt

lesliecdubs avatar Oct 24 '22 22:10 lesliecdubs