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 @@ -23,5 +23,6 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
### Code quality

- Upgraded the `Banner`, `Card`, and `Modal` components from legacy context API to use createContext ([#786](https://github.com/Shopify/polaris-react/pull/786))
- Refactored `Frame` and its subcomponents to use the `createContext` API instead of legacy context ([#803](https://github.com/Shopify/polaris-react/pull/803))

### Deprecations
39 changes: 24 additions & 15 deletions src/components/ContextualSaveBar/ContextualSaveBar.tsx
Original file line number Diff line number Diff line change
@@ -1,32 +1,38 @@
import * as React from 'react';
import compose from '@shopify/react-compose';
import isEqual from 'lodash/isEqual';
import {
ContextualSaveBarProps,
FrameContext,
frameContextTypes,
} from '../Frame';
import {ContextualSaveBarProps, FrameContext, Consumer} from '../Frame';
import withContext from '../WithContext';
import {WithContextTypes} from '../../types';
import {withAppProvider, WithAppProviderProps} from '../AppProvider';

// The script in the styleguide that generates the Props Explorer data expects
// a component's props to be found in the Props interface. This silly workaround
// ensures that the Props Explorer table is generated correctly, instead of
// crashing if we write `ContextualSaveBar extends React.Component<ContextualSaveBarProps>`
interface Props extends ContextualSaveBarProps {}
export type ComposedProps = Props &
WithAppProviderProps &
WithContextTypes<FrameContext>;

class ContextualSaveBar extends React.PureComponent<Props, never> {
static contextTypes = frameContextTypes;
context: FrameContext;

class ContextualSaveBar extends React.PureComponent<ComposedProps, never> {
componentDidMount() {
this.context.frame.setContextualSaveBar(this.props);
const {
props: {polaris, context, ...rest},
} = this;
context.frame.setContextualSaveBar(rest);
}

componentWillUnmount() {
this.context.frame.removeContextualSaveBar();
this.props.context.frame.removeContextualSaveBar();
}

componentDidUpdate(oldProps: Props) {
if (contextualSaveBarHasChanged(this.props, oldProps)) {
this.context.frame.setContextualSaveBar(this.props);
componentDidUpdate(oldProps: ComposedProps) {
const {
props: {polaris, context, ...rest},
} = this;
if (contextualSaveBarHasChanged(rest, oldProps)) {
context.frame.setContextualSaveBar(rest);
}
}

Expand All @@ -50,4 +56,7 @@ function contextualSaveBarHasChanged(
);
}

export default ContextualSaveBar;
export default compose<Props>(
withContext<Props, WithAppProviderProps, FrameContext>(Consumer),
withAppProvider(),
)(ContextualSaveBar);
93 changes: 61 additions & 32 deletions src/components/ContextualSaveBar/tests/ContextualSaveBar.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import * as React from 'react';
import * as PropTypes from 'prop-types';
import {mountWithAppProvider} from 'test-utilities';
import {noop} from '../../../utilities/other';
import ContextualSaveBar from '../ContextualSaveBar';
import {Provider, createFrameContext} from '../../Frame';

describe('<ContextualSaveBar />', () => {
const props = {
Expand All @@ -12,58 +12,87 @@ describe('<ContextualSaveBar />', () => {
};

it('calls the contextual save bar on mount with correct values', () => {
const {frame} = mountWithContext(<ContextualSaveBar {...props} />);
expect(frame.setContextualSaveBar).toHaveBeenCalledWith({
const mockFrameContext = createFrameContext({
setContextualSaveBar: jest.fn(),
removeContextualSaveBar: jest.fn(),
});

mountWithAppProvider(
<Provider value={mockFrameContext}>
<ContextualSaveBar {...props} />
</Provider>,
);
expect(mockFrameContext.frame.setContextualSaveBar).toHaveBeenCalledWith({
...props,
});
});

it('removes the contextual save bar on unmount', () => {
const {contextualSaveBar, frame} = mountWithContext(
<ContextualSaveBar {...props} />,
const mockFrameContext = createFrameContext({
setContextualSaveBar: jest.fn(),
removeContextualSaveBar: jest.fn(),
});

const frame = mountWithAppProvider(
<Provider value={mockFrameContext}>
<ContextualSaveBar {...props} />
</Provider>,
);
expect(frame.removeContextualSaveBar).not.toHaveBeenCalled();
contextualSaveBar.unmount();
expect(frame.removeContextualSaveBar).toHaveBeenCalled();
expect(
mockFrameContext.frame.removeContextualSaveBar,
).not.toHaveBeenCalled();
frame.unmount();
expect(mockFrameContext.frame.removeContextualSaveBar).toHaveBeenCalled();
});

it('calls the contextual save bar with correct values if its props change after it mounted', () => {
const {frame, contextualSaveBar} = mountWithContext(
<ContextualSaveBar {...props} />,
const mockFrameContext = createFrameContext({
setContextualSaveBar: jest.fn(),
removeContextualSaveBar: jest.fn(),
});

const frame = mountWithAppProvider(
<Provider value={mockFrameContext}>
<ContextualSaveBar {...props} />
</Provider>,
);
expect(frame.setContextualSaveBar).toHaveBeenCalledTimes(1);
const newProps = {
saveAction: {content: 'Save', onAction: noop, loading: true},
discardAction: {content: 'Discard', onAction: noop},
message: 'Unsaved changes',
};
contextualSaveBar.setProps({...newProps});
expect(frame.setContextualSaveBar).toHaveBeenCalledWith({
frame.setProps({
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, why do we need to setProps on frame now?

Copy link
Author

Choose a reason for hiding this comment

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

This is an alternative to mounting and unmounting the child component, since we no longer have direct access to the child component. Calling setProps on Frame with children that have new props triggers the componentDidUpdate lifecycle method for those children.

children: <ContextualSaveBar {...newProps} />,
});
expect(mockFrameContext.frame.setContextualSaveBar).toHaveBeenCalledWith({
...newProps,
});
expect(frame.setContextualSaveBar).toHaveBeenCalledTimes(2);
expect(mockFrameContext.frame.setContextualSaveBar).toHaveBeenCalledTimes(
2,
);
});

it('doesnt call the contextual save bar if its props remain unchanged after it mounted', () => {
const {frame, contextualSaveBar} = mountWithContext(
<ContextualSaveBar {...props} />,
const mockFrameContext = createFrameContext({
setContextualSaveBar: jest.fn(),
removeContextualSaveBar: jest.fn(),
});

const frame = mountWithAppProvider(
<Provider value={mockFrameContext}>
<ContextualSaveBar {...props} />
</Provider>,
);

expect(mockFrameContext.frame.setContextualSaveBar).toHaveBeenCalledTimes(
1,
);
expect(frame.setContextualSaveBar).toHaveBeenCalledTimes(1);
const newProps = {...props};
contextualSaveBar.setProps({...newProps});
expect(frame.setContextualSaveBar).toHaveBeenCalledTimes(1);
frame.setProps({
children: <ContextualSaveBar {...newProps} />,
});
expect(mockFrameContext.frame.setContextualSaveBar).toHaveBeenCalledTimes(
1,
);
});
});

function mountWithContext(element: React.ReactElement<any>) {
const frame = {
setContextualSaveBar: jest.fn(),
removeContextualSaveBar: jest.fn(),
};
const contextualSaveBar = mountWithAppProvider(element, {
context: {frame},
childContextTypes: {frame: PropTypes.any},
});

return {contextualSaveBar, frame};
}
85 changes: 43 additions & 42 deletions src/components/Frame/Frame.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import {setRootProperty} from '../../utilities/setRootProperty';
import {
ContextualSaveBarProps,
FrameContext,
frameContextTypes,
ToastProps,
ToastID,
ToastPropsWithID,
} from './types';
import {ToastManager, Loading, ContextualSaveBar} from './components';
import {Provider, ToastManager, Loading, ContextualSaveBar} from './components';

import * as styles from './Frame.scss';

Expand All @@ -41,7 +41,7 @@ export interface State {
skipFocused?: boolean;
globalRibbonHeight: number;
loadingStack: number;
toastMessages: (ToastProps & {id: string})[];
toastMessages: (ToastPropsWithID)[];
showContextualSaveBar: boolean;
}

Expand All @@ -54,8 +54,6 @@ const APP_FRAME_LOADING_BAR = 'AppFrameLoadingBar';
export type CombinedProps = Props & WithAppProviderProps;

export class Frame extends React.PureComponent<CombinedProps, State> {
static childContextTypes = frameContextTypes;

state: State = {
skipFocused: false,
globalRibbonHeight: 0,
Expand All @@ -69,19 +67,6 @@ export class Frame extends React.PureComponent<CombinedProps, State> {

private globalRibbonContainer: HTMLDivElement | null = null;

getChildContext(): FrameContext {
return {
frame: {
showToast: this.showToast,
hideToast: this.hideToast,
startLoading: this.startLoading,
stopLoading: this.stopLoading,
setContextualSaveBar: this.setContextualSaveBar,
removeContextualSaveBar: this.removeContextualSaveBar,
},
};
}

componentDidMount() {
this.handleResize();
if (this.props.globalRibbon) {
Expand Down Expand Up @@ -240,28 +225,30 @@ export class Frame extends React.PureComponent<CombinedProps, State> {
) : null;

return (
<div
className={frameClassName}
{...layer.props}
{...navigationAttributes}
>
{skipMarkup}
{topBarMarkup}
{contextualSaveBarMarkup}
{loadingMarkup}
{navigationOverlayMarkup}
{navigationMarkup}
<main
className={styles.Main}
id={APP_FRAME_MAIN}
data-has-global-ribbon={Boolean(globalRibbon)}
<Provider value={this.getContext}>
<div
className={frameClassName}
{...layer.props}
{...navigationAttributes}
>
<div className={styles.Content}>{children}</div>
</main>
<ToastManager toastMessages={toastMessages} />
{globalRibbonMarkup}
<EventListener event="resize" handler={this.handleResize} />
</div>
{skipMarkup}
{topBarMarkup}
{contextualSaveBarMarkup}
{loadingMarkup}
{navigationOverlayMarkup}
{navigationMarkup}
<main
className={styles.Main}
id={APP_FRAME_MAIN}
data-has-global-ribbon={Boolean(globalRibbon)}
>
<div className={styles.Content}>{children}</div>
</main>
<ToastManager toastMessages={toastMessages} />
{globalRibbonMarkup}
<EventListener event="resize" handler={this.handleResize} />
</div>
</Provider>
);
}

Expand Down Expand Up @@ -289,7 +276,7 @@ export class Frame extends React.PureComponent<CombinedProps, State> {
}

@autobind
private showToast(toast: {id: string} & ToastProps) {
private showToast(toast: ToastPropsWithID) {
this.setState(({toastMessages}: State) => {
const hasToastById =
toastMessages.find(({id}) => id === toast.id) != null;
Expand All @@ -300,7 +287,7 @@ export class Frame extends React.PureComponent<CombinedProps, State> {
}

@autobind
private hideToast({id}: {id: string}) {
private hideToast({id}: ToastID) {
this.setState(({toastMessages}: State) => {
return {
toastMessages: toastMessages.filter(({id: toastId}) => id !== toastId),
Expand Down Expand Up @@ -390,6 +377,20 @@ export class Frame extends React.PureComponent<CombinedProps, State> {
this.handleNavigationDismiss();
}
}

@autobind
get getContext(): FrameContext {
return {
frame: {
showToast: this.showToast,
hideToast: this.hideToast,
startLoading: this.startLoading,
stopLoading: this.stopLoading,
setContextualSaveBar: this.setContextualSaveBar,
removeContextualSaveBar: this.removeContextualSaveBar,
},
};
}
}

const navTransitionClasses = {
Expand Down
18 changes: 18 additions & 0 deletions src/components/Frame/components/Context/Context.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import * as React from 'react';
import {noop} from '@shopify/javascript-utilities/other';
import {FrameContext} from '../../types';

const defaultContext: FrameContext = {
frame: {
showToast: noop,
hideToast: noop,
setContextualSaveBar: noop,
removeContextualSaveBar: noop,
startLoading: noop,
stopLoading: noop,
},
};

const {Provider, Consumer} = React.createContext<FrameContext>(defaultContext);

export {Provider, Consumer};
1 change: 1 addition & 0 deletions src/components/Frame/components/Context/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export {Provider, Consumer} from './Context';
4 changes: 2 additions & 2 deletions src/components/Frame/components/ToastManager/ToastManager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import {autobind} from '@shopify/javascript-utilities/decorators';
import {classNames} from '@shopify/react-utilities/styles';
import EventListener from '../../../EventListener';
import Portal from '../../../Portal';
import {ToastProps} from '../../types';
import {ToastPropsWithID} from '../../types';
import Toast from '../Toast';

import * as styles from './ToastManager.scss';

export interface Props {
toastMessages: (ToastProps & {id: string})[];
toastMessages: (ToastPropsWithID)[];
}

export default class ToastManager extends React.PureComponent<Props, never> {
Expand Down
1 change: 1 addition & 0 deletions src/components/Frame/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export {
} from './ToastManager';
export {default as Loading, Props as LoadingProps} from './Loading';
export {default as ContextualSaveBar} from './ContextualSaveBar';
export {Provider, Consumer} from './Context';
Loading