Skip to content
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
1 change: 1 addition & 0 deletions UNRELEASED-V4.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
### Code quality

- Added hooks to storybooks scope ([#1672](https://github.com/Shopify/polaris-react/pull/1672))
- Removed all uses of `ReactDOM.findDOMNode` ([#1696](https://github.com/Shopify/polaris-react/pull/1696))
- Simplified `WithinContentContainer` context type ([#1602](https://github.com/Shopify/polaris-react/pull/1602))
- Updated `Collapsible` to no longer use `componentWillReceiveProps`([#1670](https://github.com/Shopify/polaris-react/pull/1670))
- Remove `withRef` and `withContext` from `DropZone.FileUpload` ([#1491](https://github.com/Shopify/polaris-react/pull/1491))
Expand Down
6 changes: 3 additions & 3 deletions src/components/Focus/Focus.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import React from 'react';
import {findDOMNode} from 'react-dom';
import isEqual from 'lodash/isEqual';
import {focusFirstFocusableNode} from '@shopify/javascript-utilities/focus';

export interface Props {
children?: React.ReactNode;
disabled?: boolean;
root: HTMLElement | null;
}

export default class Focus extends React.PureComponent<Props, never> {
Expand All @@ -24,11 +24,11 @@ export default class Focus extends React.PureComponent<Props, never> {
}

handleSelfFocus() {
if (this.props.disabled) {
const {disabled, root} = this.props;
if (disabled) {
return;
}

const root = findDOMNode(this) as HTMLElement | null;
if (root) {
if (!root.querySelector('[autofocus]')) {
focusFirstFocusableNode(root, false);
Expand Down
50 changes: 28 additions & 22 deletions src/components/Focus/tests/Focus.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import React, {useRef, useState, useEffect} from 'react';
import {mountWithAppProvider} from 'test-utilities/legacy';
import Focus from '../Focus';
import Focus, {Props} from '../Focus';
import {Discard} from '../../../types';

describe('<Focus />', () => {
let requestAnimationFrameSpy: jest.SpyInstance;
Expand All @@ -15,34 +16,26 @@ describe('<Focus />', () => {
});

it('mounts', () => {
const focus = mountWithAppProvider(
<Focus>
<div />
</Focus>,
);
const focus = mountWithAppProvider(<FocusTestWrapper />);

expect(focus.exists()).toBe(true);
});

it('will not focus any element if none are natively focusable', () => {
mountWithAppProvider(
<Focus>
<div>
<span />
</div>
</Focus>,
<FocusTestWrapper>
<span />
</FocusTestWrapper>,
);

expect(document.body).toBe(document.activeElement);
});

it('will focus first focusable node', () => {
const focus = mountWithAppProvider(
<Focus>
<div>
<input />
</div>
</Focus>,
<FocusTestWrapper>
<input />
</FocusTestWrapper>,
);

const input = focus.find('input').getDOMNode();
Expand All @@ -51,14 +44,27 @@ describe('<Focus />', () => {

it('will not focus the first focusable node is the `disabled` is true', () => {
const focus = mountWithAppProvider(
<Focus disabled>
<div>
<input />
</div>
</Focus>,
<FocusTestWrapper disabled>
<input />
</FocusTestWrapper>,
);

const input = focus.find('input').getDOMNode();
expect(input).not.toBe(document.activeElement);
});
});

function FocusTestWrapper({children, ...props}: Discard<Props, 'root'>) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Wrapper component that passes a root to Focus

const root = useRef<HTMLDivElement>(null);
const [, setMount] = useState(false);

useEffect(() => {
setMount(true);
}, []);

return (
<Focus {...props} root={root.current}>
<div ref={root}>{children}</div>
</Focus>
);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from 'react';
import {findDOMNode} from 'react-dom';
import React, {createRef} from 'react';
import {CaretDownMinor} from '@shopify/polaris-icons';

import {classNames} from '../../../../../../utilities/css';
Expand All @@ -20,14 +19,13 @@ export default class BulkActionButton extends React.PureComponent<
Props,
never
> {
private bulkActionButton = createRef<HTMLButtonElement>();

componentDidMount() {
const {handleMeasurement} = this.props;
if (handleMeasurement) {
const bulkActionButtonNode = findDOMNode(this);
const width =
(bulkActionButtonNode instanceof Element &&
bulkActionButtonNode.getBoundingClientRect().width) ||
0;
const width = (this.bulkActionButton
.current as HTMLButtonElement).getBoundingClientRect().width;
Copy link
Member Author

Choose a reason for hiding this comment

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

This cast is to increase codecov. We don't optionally render the button so this ref will always exist in componentDidMount lifecycle

handleMeasurement(width);
}
}
Expand Down Expand Up @@ -82,6 +80,7 @@ export default class BulkActionButton extends React.PureComponent<
aria-label={accessibilityLabel}
type="button"
disabled={disabled}
ref={this.bulkActionButton}
>
{contentMarkup}
</button>
Expand Down
5 changes: 2 additions & 3 deletions src/components/Tabs/components/TabMeasurer/TabMeasurer.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from 'react';
import {findDOMNode} from 'react-dom';
import {classNames} from '../../../../utilities/css';
import EventListener from '../../../EventListener';

Expand Down Expand Up @@ -89,9 +88,9 @@ export default class TabMeasurer extends React.PureComponent<Props, never> {

const {handleMeasurement} = this.props;
const containerWidth = this.containerNode.offsetWidth;
const tabMeasurerNode = findDOMNode(this);

const hiddenTabNodes =
tabMeasurerNode instanceof Element && tabMeasurerNode.children;
this.containerNode instanceof Element && this.containerNode.children;
const hiddenTabNodesArray: HTMLElement[] = [].slice.call(hiddenTabNodes);
const hiddenTabWidths = hiddenTabNodesArray.map((node) => {
return node.getBoundingClientRect().width;
Expand Down
2 changes: 1 addition & 1 deletion src/components/TrapFocus/TrapFocus.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default class TrapFocus extends React.PureComponent<Props, State> {
const {children} = this.props;

return (
<Focus disabled={this.shouldDisable}>
<Focus disabled={this.shouldDisable} root={this.focusTrapWrapper}>
<div ref={this.setFocusTrapWrapper}>
<EventListener event="focusout" handler={this.handleBlur} />
{children}
Expand Down
2 changes: 2 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,3 +258,5 @@ export type DeepPartial<T> = {
? ReadonlyArray<DeepPartial<U>>
: DeepPartial<T[P]>
};

export type Discard<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>>;