Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
ba14e65
preventing text selection on all pressable elements
LFDanLu Oct 15, 2021
9a19db9
allowing users to select link text if click+drag starts from off the …
LFDanLu Oct 16, 2021
202ade1
moving shared logic out of usePress and into textSelection
LFDanLu Oct 18, 2021
acfe3ad
Merge branch 'main' into user_select_none_fix
LFDanLu Oct 18, 2021
f8bc103
add comment
LFDanLu Oct 18, 2021
243d814
Merge branch 'user_select_none_fix' of github.com:adobe/react-spectru…
LFDanLu Oct 18, 2021
e73fb52
fixing textselection cleanup on multipress for non iOS and adding test
LFDanLu Oct 18, 2021
3cfd01d
review comments
LFDanLu Oct 26, 2021
15dbd3b
Merge branch 'main' into user_select_none_fix
LFDanLu Oct 26, 2021
35eb3c5
addressing review comments
LFDanLu Oct 27, 2021
008c22f
Merge branch 'main' into user_select_none_fix
LFDanLu Nov 13, 2021
64058f0
Merge branch 'main' into user_select_none_fix
LFDanLu Nov 13, 2021
8198200
if user select is no longer none upon restore selection, dont overwri…
LFDanLu Nov 17, 2021
1304695
example to test rerender during button press
LFDanLu Nov 17, 2021
ce28bd7
fix lint
LFDanLu Nov 17, 2021
9270d28
fix lint for sure
LFDanLu Nov 17, 2021
1e8b22b
Adding tests for style changes during press down
LFDanLu Nov 17, 2021
c62a7a1
fixing lint
LFDanLu Nov 17, 2021
a57144b
Merge branch 'main' into user_select_none_fix
LFDanLu Nov 18, 2021
fd04343
Merge branch 'main' into user_select_none_fix
LFDanLu Nov 18, 2021
b7e4564
Merge branch 'main' into user_select_none_fix
LFDanLu Nov 18, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 57 additions & 34 deletions packages/@react-aria/interactions/src/textSelection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,52 +21,75 @@ import {isIOS, runAfterTransition} from '@react-aria/utils';
// There are three possible states due to the delay before removing user-select: none after
// pointer up. The 'default' state always transitions to the 'disabled' state, which transitions
// to 'restoring'. The 'restoring' state can either transition back to 'disabled' or 'default'.

// For non-iOS devices, we apply user-select: none to the pressed element instead to avoid possible
// performance issues that arise from applying and removing user-select: none to the entire page
// (see https://github.com/adobe/react-spectrum/issues/1609).
type State = 'default' | 'disabled' | 'restoring';

// Note that state only matters here for iOS. Non-iOS gets user-select: none applied to the target element
// rather than at the document level so we just need to apply/remove user-select: none for each pressed element individually
let state: State = 'default';
let savedUserSelect = '';
let modifiedElementMap = new WeakMap<HTMLElement, string>();

export function disableTextSelection() {
// Limit this behavior to iOS only. Android devices don't text select nearby element
// when long pressing on a different element.
if (!isIOS()) {
return;
}
export function disableTextSelection(target?: HTMLElement) {
if (isIOS()) {
if (state === 'default') {
savedUserSelect = document.documentElement.style.webkitUserSelect;
document.documentElement.style.webkitUserSelect = 'none';
}

if (state === 'default') {
savedUserSelect = document.documentElement.style.webkitUserSelect;
document.documentElement.style.webkitUserSelect = 'none';
state = 'disabled';
} else if (target) {
// If not iOS, store the target's original user-select and change to user-select: none
// Ignore state since it doesn't apply for non iOS
modifiedElementMap.set(target, target.style.userSelect);
target.style.userSelect = 'none';
Comment thread
devongovett marked this conversation as resolved.
}

state = 'disabled';
}

export function restoreTextSelection() {
// If the state is already default, there's nothing to do.
// If it is restoring, then there's no need to queue a second restore.
// Limit this behavior to iOS only. Android devices don't text select nearby element
// when long pressing on a different element.
if (state !== 'disabled' || !isIOS()) {
return;
}
export function restoreTextSelection(target?: HTMLElement) {
if (isIOS()) {
// If the state is already default, there's nothing to do.
// If it is restoring, then there's no need to queue a second restore.
if (state !== 'disabled') {
Comment thread
snowystinger marked this conversation as resolved.
return;
}

state = 'restoring';
state = 'restoring';

// There appears to be a delay on iOS where selection still might occur
// after pointer up, so wait a bit before removing user-select.
setTimeout(() => {
// Wait for any CSS transitions to complete so we don't recompute style
// for the whole page in the middle of the animation and cause jank.
runAfterTransition(() => {
// Avoid race conditions
if (state === 'restoring') {
if (document.documentElement.style.webkitUserSelect === 'none') {
document.documentElement.style.webkitUserSelect = savedUserSelect || '';
// There appears to be a delay on iOS where selection still might occur
// after pointer up, so wait a bit before removing user-select.
setTimeout(() => {
// Wait for any CSS transitions to complete so we don't recompute style
// for the whole page in the middle of the animation and cause jank.
runAfterTransition(() => {
// Avoid race conditions
if (state === 'restoring') {
if (document.documentElement.style.webkitUserSelect === 'none') {
document.documentElement.style.webkitUserSelect = savedUserSelect || '';
}

savedUserSelect = '';
state = 'default';
}
});
}, 300);
} else {
// If not iOS, restore the target's original user-select if any
// Ignore state since it doesn't apply for non iOS
if (target && modifiedElementMap.has(target)) {
let targetOldUserSelect = modifiedElementMap.get(target);

if (target.style.userSelect === 'none') {
target.style.userSelect = targetOldUserSelect;
}

savedUserSelect = '';
state = 'default';
if (target.getAttribute('style') === '') {
target.removeAttribute('style');
}
});
}, 300);
modifiedElementMap.delete(target);
}
}
}
37 changes: 28 additions & 9 deletions packages/@react-aria/interactions/src/usePress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ export interface PressProps extends PressEvents {
* still pressed, onPressStart will be fired again. If set to `true`, the press is canceled
* when the pointer leaves the target and onPressStart will not be fired if the pointer returns.
*/
shouldCancelOnPointerExit?: boolean
shouldCancelOnPointerExit?: boolean,
/** Whether text selection should be enabled on the pressable element. */
allowTextSelectionOnPress?: boolean
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a case where the user has this set to false, interacts with the page, the user or the app sets this to true and things don't perform (restore/disable) correctly?

Also, this seems look a cool feature, but what components are using it?

Copy link
Copy Markdown
Member Author

@LFDanLu LFDanLu Nov 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So by default we want this behavior for our RSP components (aka prevent text selection on press so Link/Accordion text doesn't get highlighted on long press). This option is introduced so people can opt out of this behavior. As for the case you described, it should get cleaned up here

useEffect(() => {
return () => {
if (!allowTextSelectionOnPress) {
restoreTextSelection(ref.current.target);
}
};
}, [allowTextSelectionOnPress]);

The allowTextSelectionOnPress in the if statement will be false since the value was declared in the previous render before the change to allowTextSelectionOnPress=true and thus will clean up the user-select stuff

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we have a specific use case for this option so far?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devongovett No specific use case yet, added it here cuz of what we discussed in our original slack convo. I can go ahead and remove it for now and we can introduce it if people request it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brought allowTextSelectionOnPress back when reverting to this PR's original approach. I felt like this will allow someone to opt out of our user-select: none modification if they want to handle it themselves or if they have a good reason to not apply that to a pressable element. Happy to remove it if people still don't want this prop

}

export interface PressHookProps extends PressProps {
Expand Down Expand Up @@ -99,6 +101,7 @@ export function usePress(props: PressHookProps): PressResult {
isPressed: isPressedProp,
preventFocusOnPress,
shouldCancelOnPointerExit,
allowTextSelectionOnPress,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
ref: _, // Removing `ref` from `domProps` because TypeScript is dumb,
...domProps
Expand Down Expand Up @@ -217,7 +220,9 @@ export function usePress(props: PressHookProps): PressResult {
state.activePointerId = null;
state.pointerType = null;
removeAllGlobalListeners();
restoreTextSelection();
if (!allowTextSelectionOnPress) {
restoreTextSelection(state.target);
}
}
};

Expand Down Expand Up @@ -329,7 +334,10 @@ export function usePress(props: PressHookProps): PressResult {
focusWithoutScrolling(e.currentTarget);
}

disableTextSelection();
if (!allowTextSelectionOnPress) {
disableTextSelection(state.target);
}

triggerPressStart(e, state.pointerType);

addGlobalListener(document, 'pointermove', onPointerMove, false);
Expand Down Expand Up @@ -404,7 +412,9 @@ export function usePress(props: PressHookProps): PressResult {
state.activePointerId = null;
state.pointerType = null;
removeAllGlobalListeners();
restoreTextSelection();
if (!allowTextSelectionOnPress) {
restoreTextSelection(state.target);
}
}
};

Expand Down Expand Up @@ -535,7 +545,10 @@ export function usePress(props: PressHookProps): PressResult {
focusWithoutScrolling(e.currentTarget);
}

disableTextSelection();
if (!allowTextSelectionOnPress) {
disableTextSelection(state.target);
}

triggerPressStart(e, state.pointerType);

addGlobalListener(window, 'scroll', onScroll, true);
Expand Down Expand Up @@ -588,7 +601,9 @@ export function usePress(props: PressHookProps): PressResult {
state.activePointerId = null;
state.isOverTarget = false;
state.ignoreEmulatedMouseEvents = true;
restoreTextSelection();
if (!allowTextSelectionOnPress) {
restoreTextSelection(state.target);
}
removeAllGlobalListeners();
};

Expand Down Expand Up @@ -625,13 +640,17 @@ export function usePress(props: PressHookProps): PressResult {
}

return pressProps;
}, [addGlobalListener, isDisabled, preventFocusOnPress, removeAllGlobalListeners]);
}, [addGlobalListener, isDisabled, preventFocusOnPress, removeAllGlobalListeners, allowTextSelectionOnPress]);

// Remove user-select: none in case component unmounts immediately after pressStart
// eslint-disable-next-line arrow-body-style
useEffect(() => {
return () => restoreTextSelection();
}, []);
return () => {
if (!allowTextSelectionOnPress) {
restoreTextSelection(ref.current.target);
Comment thread
LFDanLu marked this conversation as resolved.
}
};
}, [allowTextSelectionOnPress]);

return {
isPressed: isPressedProp || isPressed,
Expand Down
Loading