Skip to content

Commit

Permalink
drag handle no longer stealing focus on mount from autoFocus elements
Browse files Browse the repository at this point in the history
  • Loading branch information
alexreardon committed Apr 24, 2019
1 parent 164ffa1 commit d15bb62
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 1 deletion.
15 changes: 14 additions & 1 deletion src/view/use-drag-handle/use-focus-retainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ export default function useFocusRetainer(args: Args): Result {
isFocusedRef.current = false;
}, []);

// This effect handles:
// - giving focus on mount
// - registering focus on unmount
useIsomorphicLayoutEffect(() => {
// mounting: try to restore focus
const first: Args = lastArgsRef.current;
Expand Down Expand Up @@ -63,9 +66,19 @@ export default function useFocusRetainer(args: Args): Result {
};
}, [getDraggableRef]);

const lastDraggableRef = useRef<?HTMLElement>(getDraggableRef());
// will always be null on the first render as nothing has mounted yet
const lastDraggableRef = useRef<?HTMLElement>(null);

// This effect restores focus to an element when a
// ref changes while a component is still mounted.
// This can happen when a drag handle is moved into a portal
useIsomorphicLayoutEffect(() => {
// this can happen on the first mount - no draggable ref is set
// this effect is not handling initial mounting
if (!lastDraggableRef.current) {
return;
}

const draggableRef: ?HTMLElement = getDraggableRef();

// Cannot focus on nothing
Expand Down
51 changes: 51 additions & 0 deletions test/unit/view/drag-handle/focus-management.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import React, { type Node } from 'react';
import invariant from 'tiny-invariant';
import ReactDOM from 'react-dom';
import { Simulate } from 'react-dom/test-utils';
import { mount } from 'enzyme';
import type { ReactWrapper } from 'enzyme';
import type { DragHandleProps } from '../../../../src/view/use-drag-handle/drag-handle-types';
Expand All @@ -12,6 +13,7 @@ import forceUpdate from '../../../utils/force-update';
import AppContext, {
type AppContextValue,
} from '../../../../src/view/context/app-context';
import createRef from '../../../utils/create-ref';

const body: ?HTMLElement = document.body;
invariant(body, 'Cannot find body');
Expand Down Expand Up @@ -385,4 +387,53 @@ describe('Focus retention moving between lists (focus retention between mounts)'
// focus maintained on button
expect(button).toBe(document.activeElement);
});

it('should not steal focus from an element with auto focus on mount', () => {
function App() {
const ref = createRef();
return (
<AppContext.Provider value={appContext}>
<WithDragHandle
draggableId="draggable"
callbacks={getStubCallbacks()}
isDragging={false}
isDropAnimating={false}
isEnabled
getDraggableRef={ref.getRef}
canDragInteractiveElements={false}
getShouldRespectForcePress={() => true}
>
{(dragHandleProps: ?DragHandleProps) => (
<div
ref={ref.setRef}
{...dragHandleProps}
className="drag-handle"
>
Drag me!
{/* autoFocus attribute does give focus, but does not trigger onFocus callback */}
<textarea
ref={(el: ?HTMLElement) => {
// Replicating auto focus
// by leveraging the ref function.
invariant(
el instanceof HTMLElement,
'Expected el to be a html element',
);
el.focus();
// letting react know about it
Simulate.focus(el);
}}
/>
</div>
)}
</WithDragHandle>
</AppContext.Provider>
);
}

const wrapper: ReactWrapper<*> = mount(<App />);

// asserting drag handle did not steal focus
expect(wrapper.find('textarea').getDOMNode()).toBe(document.activeElement);
});
});

0 comments on commit d15bb62

Please sign in to comment.