[Button][material-ui] Improve buttons' text selection

[DiegoAndai]

• I have followed (at least) the PR section of the contributing guide.

Following up on Point 13 of this comment: #36622 (comment).

Edited Sept. 13:

The original intent of this PR was to enhance the button's text selection for both Material UI and Joy UI. I'm changing that to include only Material UI.

Joy UI's change is more complex as the disabled styles are implemented in variantUtils.tsx and not directly in the components. That change could impact other components, so I prefer someone with more experience in the Joy UI codebase to own it. I created an issue for it: #38959

Material UI button's text selection

Preview: https://deploy-preview-38158--material-ui.netlify.app/material-ui/react-button/

The expected behavior is:

• Text selection from outside is possible
• Text selection from double-click is not possible

A technical limitation (explained in detail here) makes it difficult to achieve this when the DOM element used is not button and it's disabled. The best solution is implementing the behavior we want for all variations except for disabled non-button elements. This PR enables all selection for those elements, which is better than no selection, IMO.

You can check out the differences in this comparison experiment.

Components changed

• ButtonBase, which impacts various components like FAB, Button, and CardActionArea
• Chip (clickable)
• Link
• material-next's ButtonBase

Note 1

The origin of the user-select: none rule is this commit, which was fixing issue #8 (it goes baaaack). The issue does not come back with this PR.

Note 2

Chrome's and Safari's default behavior for buttons is that text is selectable. In Firefox, the buttons' text is not selectable.

[mui-bot]

Netlify deploy preview

https://deploy-preview-38158--material-ui.netlify.app/

@material-ui/core: parsed: +Infinity% , gzip: +Infinity%
@material-ui/lab: parsed: +Infinity% , gzip: +Infinity%
@material-ui/styles: parsed: +Infinity% , gzip: +Infinity%
@material-ui/private-theming: parsed: +Infinity% , gzip: +Infinity%
@material-ui/system: parsed: +Infinity% , gzip: +Infinity%
@material-ui/unstyled: parsed: +Infinity% , gzip: +Infinity%
@material-ui/utils: parsed: +Infinity% , gzip: +Infinity%
@mui/material-next: parsed: +Infinity% , gzip: +Infinity%
@mui/joy: parsed: +Infinity% , gzip: +Infinity%

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against a4016f1

[oliviertassinari]

A few problems I can spot to not regress on #8 (double click on button shouldn't select it) where I think that we still need the style:

• Material UI Button disabled={true}
• Joy UI Button component="div" and any none button elements
• Joy UI Button disabled={true}

[DiegoAndai]

@oliviertassinari I'm not convinced we should do so. I lean towards relying on the browser's default, my reasoning (tested on Chrome):

Regarding disabled

This one is interesting, you'll notice that Base's disabled button does not select on double click, but Joy's does. This difference is due to the pointer-events: none attribute on Joy's Button (coming from here). If you add that attribute to Base's, then the text is selectable by double clicking.

I think the pointer-events: none attribute is correct for a disabled button, and Base's example should include it.

• Option one is only having pointer-events: none and relying on whatever the browser decides regarding text selection.
• Option two would be to add user-select: none whenever we add pointer-events: none.

I lean towards option one, but not 100% convinced.

Regarding component="div"

I think it's expected that changing the root component to div would make the text selectable on double click as that's the behavior for divs, so whatever component is used should maintain its default. This is the case for Base's button if you provide slots={{ root: 'div' }}.

Benchmarks

• Shadcn disables double click text selection on the enabled button, but allows for it on the disabled button (as it adds the pointer-events: none attribute). It allows dragging text selection for both. Changing the root component to div enables text selection on both states.
• Chakra and Mantine disable text selection completely on their Buttons, exactly as we have in Material right now. Changing the root component to div doesn't change anything and text is still not selectable.

"Breaking change"

I do admit that my proposal might be perceived as a breaking change for users, and if that's the case then I would say yes to your considerations for v5, but I wouldn't do so for v6.

What do you think?

[oliviertassinari]

I lean towards relying on the browser's default

@DiegoAndai This is where I'm coming from as well. The cases I raise are cases where we don't respect the browser's default, which is if I look at Chrome, Firefox, Safari, on macOS and Windows, in the majority vote:

• double click doesn't select text
• text selection is possible, starting from the outside.

and this regardless of the disabled state (it's still a button element you might try to spam click on) or the host element (it's still a role="button").

I do admit that my proposal might be perceived as a breaking change for users, and if that's the case then I would say yes to your considerations for v5, but I wouldn't do so for v6.

We have done way bolder design changes in the past without pushbacks, I think it's safe for a patch release (bug fix).

[oliviertassinari]

This connects to #20700 as well.

[DiegoAndai]

@oliviertassinari you convinced me, I'll implement these changes

Regarding the host component, do you think using the role is the way to go to decide regarding the behavior? meaning if the role is "button", then we should apply button text selection rules?

[oliviertassinari]

Regarding the host component, do you think using the role is the way to go to decide regarding the behavior? meaning if the role is "button", then we should apply button text selection rules?

@DiegoAndai I think that it boils down to the onClick handler, for case people double click too quickly. The role button sounds like a good way to identify it 👍. For example, if it's a link behavior that looks like a button, what would we want? I guess the regular double click text selection.

[DiegoAndai]

I've been able to implement the expected behavior:

• Double click doesn't select text
• Text selection is possible, starting from the outside

For the Material Button in the following cases:

• Any element ('button', 'a', 'div', ...) with disabled={false}
• 'button' element with disabled={true} (handled by the native button)

Non 'button' elements with disabled={true} is tricky because:

• We need the CSS style pointer-events: none to disable the pointer events. With this style, text selection by double click is enabled, and I haven't figured out how to disable it without removing pointer-events: none.
• We cannot remove pointer-events: none unless we have another way to disable pointer events. We could disable pointer events by overriding each event handler, but that is a bigger refactor

You can check what I've got so far here: https://deploy-preview-38158--material-ui.netlify.app/experiments/material/button-select

@oliviertassinari I'm stuck here, any ideas? If you want to check out how I implemented the behavior, it's in ButtonBase.js.

[oliviertassinari]

@oliviertassinari I'm stuck here, any ideas?

I wouldn't go further. I think it's as good as it can be.

I could spot two other things:

1. Joy UI <Button component="span" should get pointer-events, the same logic as Material UI IMHO.
2. We could use a :not([type="button"]) CSS selector, but it would be a breaking change, increased specificity, so probably not ideal:

https://specificity.keegan.st/

[oliviertassinari]

A guess it's why we do this?

Suggested change

	pointerEvents: 'none', // Disable link interactions
	pointerEvents: 'none', // Prevent double click text selection

[DiegoAndai]

It's the other way around. pointer-events: none is used here to turn off pointer events (on-click, on-mouse-down, etc.) in case the element is not a button (the button element disables them natively when disabled).

The side effect is that double-click selection is enabled, which is unfortunate. I couldn't find a workaround for this.

So the comment "Disable link interactions" is correct. It could be more general "Disable interactions" as it does apply for any non button element.

[DiegoAndai]

Clarification: pointer-events: none does not enable double-click selection by itself but rather prevents us from handling text selection in non-button elements:

To turn off only double-click selection, we can use

.&:active {
    user-select: none
};

This way, user selection is only disabled when clicking (active state), but not when dragging from outside the element. But with pointer-events: none, we don't get the active state, so we cannot use this trick.

Using user-select: none without the active state selector is not a good option as that turns off both double-click and selection dragging from outside.

And this is what I couldn't find a workaround for. The best (not perfect) solution would be having the behavior we want for all variations except for disabled non-button elements. Those are the ones that require pointer-events: none. For those, I propose enabling all selection, which is better than no selection, IMO.

[DiegoAndai]

Also, pointer-events: none does break the native button handling of text selection, enabling double-click selection, so we shouldn't add pointer-events: none to button elements. It's also not necessary on these, as pointer events are turned off natively.

[DiegoAndai]

I'm splitting this PR only to include Material UI and created an issue for Joy UI: #38959.

I added the explanation in this PR's description, but TL;DR is that Joy UI's change is more complex. It's worth it to improve Material UI's buttons separately.

[mnajdova]

Looks like we are capturing all scenarios based on manual testing, but we likely want to add some tests around this, as it's easy to break it.

[oliviertassinari]

@DiegoAndai I see you tried to push the solution above and beyond, covering more cases than I thought of initially, greedy 😁. There are limits though, I doubt we will be able to cover everything. Hopefully, there are workaround, otherwise, we could fallback a bit to cover fewer but still meaningful cases.

[oliviertassinari]

Will we ever need this?

I can think of 3 types of elements we can render:

1. <a> solved by built-in behavior.
2. <button> solved by built-in behavior.
3. regular element, e.g. <div>. Do we want to optimize for devs that do YOLO? I would expect always 1 and 2.

Suggested change

	'&:active': {
	userSelect: 'none',
	},

[DiegoAndai]

We will need this in Firefox to have the same behavior, as buttons in Firefox are not selectable by dragging from outside. We could add it inside the ownerState.component === 'button' condition, so it only applies to buttons, but I don't see the harm of supporting regular elements as well. What do you think?

[DiegoAndai]

Hey @oliviertassinari, I finally got time to update this PR and reply to your feedback. It is ready for you to review again 😊

I decreased the scope. I think you're right: "cover fewer but still meaningful cases"

[DiegoAndai]

PR got out of date as it has been on hold for a while. Closing as stale.

We'll have to revisit this later this year while working on v7.