search

AlaskaAirlines / auro-menu

Custom element that provides a list of options for the user to choose from
https://auro.alaskaair.com/components/auro/menu
Apache License 2.0
2 stars 3 forks source link

refactor(styles): remove userSelect:none from disabled menu options #91

Closed jason-capsule42 closed 2 years ago

jason-capsule42 commented 2 years ago

Alaska Airlines Pull Request

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes: #90

Summary:

Removes user-select: none; CSS from disabled menu items.

Type of change:

Please delete options that are not relevant.

Checklist:

By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.

Thank you for your submission!
-- Auro Design System Team

jason-capsule42 commented 2 years ago

What bug is this causing that requires the removal of this code?

This was a design decision - no reason to block user-select on disabled items.

Blocking user-select was never actually part of the design.

blackfalcon commented 2 years ago

@leeejune can you link to the version of the auro-menu we need to be referring to for this iteration? I am unaware of this design decision to not disable text selection.

Also, now that we bring this up... I'd argue that we should use user-select: none on all options in a menu. This is an opinion that is shared with shoelace https://shoelace.style/components/menu and Material UI https://mui.com/components/menus/

jason-capsule42 commented 2 years ago

@leeejune can you link to the version of the auro-menu we need to be referring to for this iteration? I am unaware of this design decision to not disable text selection.

Also, now that we bring this up... I'd argue that we should use user-select: none on all options in a menu. This is an opinion that is shared with shoelace https://shoelace.style/components/menu and Material UI https://mui.com/components/menus/

Our current design doesn't call for disabling the user-select.

That said, I agree with the shoelace and Material UI, in that if it is set too none it should be in all cases, not just disabled. That's the point of this PR. We aren't consistent in the current implementation and since the Figma doesn't call for it to be disabled we should not implement that rule selectively without that being part of the Design.