Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "click outside + immediate editor commit" edge case by swiching to checking for mousedown events #2415

Merged
merged 2 commits into from
May 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions src/editors/EditorContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { createPortal } from 'react-dom';
import { css } from '@linaria/core';

import type { EditorProps } from '../types';
import { useClickOutside } from '../hooks';
import { useMouseDownOutside } from '../hooks';

const editorContainer = css`
display: contents;
Expand All @@ -16,11 +16,11 @@ export default function EditorContainer<R, SR>({
onRowChange,
...props
}: EditorProps<R, SR>) {
const onClickCapture = useClickOutside(() => onRowChange(row, true));
const onMouseDownCapture = useMouseDownOutside(() => onRowChange(row, true));
if (column.editor == null) return null;

const editor = (
<div className={editorContainerClassname} onClickCapture={onClickCapture}>
<div className={editorContainerClassname} onMouseDownCapture={onMouseDownCapture}>
<column.editor row={row} column={column} onRowChange={onRowChange} {...props} />
</div>
);
Expand Down
8 changes: 4 additions & 4 deletions src/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
export * from './useCalculatedColumns';
export * from './useClickOutside';
export * from './useGridDimensions';
export * from './useViewportColumns';
export * from './useViewportRows';
export * from './useFocusRef';
export * from './useGridDimensions';
export * from './useLatestFunc';
export * from './useMouseDownOutside';
export * from './useRowSelection';
export * from './useViewportColumns';
export * from './useViewportRows';
39 changes: 19 additions & 20 deletions src/hooks/useClickOutside.ts → src/hooks/useMouseDownOutside.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useRef, useEffect } from 'react';

/**
* Detecting outside click on a react component is surprisingly hard.
* A general approach is to have a global click handler on the document
* A general approach is to have a global click handler on the window
* which checks if the click target is inside the editor container or
* not using editorContainer.contains(e.target). This approach works well
* until portals are used for editors. Portals render children into a DOM
Expand Down Expand Up @@ -37,57 +37,56 @@ import { useRef, useEffect } from 'react';
* in the DOM tree. This means a click handler can be attached on the window
* and on the editor container. The editor container can set a flag to notify
* that the click was inside the editor and the window click handler can use
* this flag to call onClickOutside. This approach however has a few caveats
* this flag to call onOutsideMouseDown. This approach however has a few caveats
* - Click handler on the window is set using window.addEventListener
* - Click handler on the editor container is set using onClick prop
* - Click handler on the editor container is set using onMouseDownCapture prop
*
* This means if a child component inside the editor calls e.stopPropagation
* then the click handler on the editor container will not be called whereas
* the document click handler will be called.
* https://github.com/facebook/react/issues/12518
*
* To solve this issue onClickCapture event is used.
* To solve this issue onMousedownCapture event is used.
*/

export function useClickOutside(onClick: () => void) {
export function useMouseDownOutside(onMouseDown: () => void) {
const frameRequestRef = useRef<number | undefined>();

function cancelAnimationFrameRequest() {
function cancelFrameRequest() {
if (typeof frameRequestRef.current === 'number') {
cancelAnimationFrame(frameRequestRef.current);
frameRequestRef.current = undefined;
}
}

// We need to prevent the `useEffect` from cleaning up between re-renders,
// as `handleDocumentClick` might otherwise miss valid click events.
// To that end we instead access the latest `onClick` prop via a ref.
const onClickRef = useRef((): void => {
// as `onWindowCaptureMouseDown` might otherwise miss valid mousedown events.
// To that end we instead access the latest `onMouseDown` prop via a ref.
const onMouseDownRef = useRef((): void => {
throw new Error('Cannot call an event handler while rendering.');
});

useEffect(() => {
onClickRef.current = onClick;
onMouseDownRef.current = onMouseDown;
});

useEffect(() => {
function onOutsideClick() {
function onOutsideMouseDown() {
frameRequestRef.current = undefined;
onClickRef.current();
onMouseDownRef.current();
}

function onWindowCaptureClick() {
cancelAnimationFrameRequest();
frameRequestRef.current = requestAnimationFrame(onOutsideClick);
function onWindowCaptureMouseDown() {
cancelFrameRequest();
frameRequestRef.current = requestAnimationFrame(onOutsideMouseDown);
}

window.addEventListener('click', onWindowCaptureClick, { capture: true });
addEventListener('mousedown', onWindowCaptureMouseDown, { capture: true });

return () => {
window.removeEventListener('click', onWindowCaptureClick, { capture: true });
cancelAnimationFrameRequest();
removeEventListener('mousedown', onWindowCaptureMouseDown, { capture: true });
cancelFrameRequest();
};
}, []);

return cancelAnimationFrameRequest;
return cancelFrameRequest;
}