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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v4][ContextualSaveBar] Remove withContext #1498

Merged
merged 2 commits into from May 29, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions UNRELEASED-V4.md
Expand Up @@ -41,6 +41,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
- Removed `withContext` from `ResourceList.FilterControl` ([#1500](https://github.com/Shopify/polaris-react/pull/1500))
- Upgraded the `Navigation` component from legacy context API to use createContext ([#1402](https://github.com/Shopify/polaris-react/pull/1402))
- Updated `ThemeProvider` to no longer use `componentWillReceiveProps`([#1254](https://github.com/Shopify/polaris-react/pull/1254))
- Removed `withContext` and `withAppProvider` from `ContextualSaveBar` ([#1498](https://github.com/Shopify/polaris-react/pull/1498))
- Removed unused context from `Scrollable` ([#1253](https://github.com/Shopify/polaris-react/pull/1253))
- Removed `withContext` from `Loading` ([#1497](https://github.com/Shopify/polaris-react/pull/1497))
- Removed `withRef` from `UnstyledLink` ([#1501](https://github.com/Shopify/polaris-react/pull/1501))
Expand Down
85 changes: 33 additions & 52 deletions src/components/ContextualSaveBar/ContextualSaveBar.tsx
@@ -1,64 +1,45 @@
import React from 'react';
import compose from '@shopify/react-compose';
import isEqual from 'lodash/isEqual';
import {ContextualSaveBarProps, FrameContextType, FrameContext} from '../Frame';
import withContext from '../WithContext';
import {WithContextTypes} from '../../types';
import {withAppProvider, WithAppProviderProps} from '../AppProvider';
import * as React from 'react';
import {ContextualSaveBarProps, FrameContext} from '../Frame';

// 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<FrameContextType>;

class ContextualSaveBar extends React.PureComponent<ComposedProps, never> {
componentDidMount() {
const {
props: {polaris, context, ...rest},
} = this;
context.setContextualSaveBar(rest);
}
export default React.memo(function ContextualSaveBar({
message,
saveAction,
discardAction,
alignContentFlush,
}: Props) {
const frame = React.useContext(FrameContext);

componentWillUnmount() {
this.props.context.removeContextualSaveBar();
}
React.useEffect(
() => {
if (!frame) return;

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

render() {
return null;
}
}
frame.setContextualSaveBar({
message,
saveAction,
discardAction,
alignContentFlush,
});
},
// eslint-disable-next-line react-hooks/exhaustive-deps
Copy link
Member Author

Choose a reason for hiding this comment

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

Frame context next changes so we don't need it as a dep

[message, saveAction, discardAction, alignContentFlush],
);

function contextualSaveBarHasChanged(
{message, saveAction, discardAction}: Props,
{
message: oldMessage,
saveAction: oldsaveAction,
discardAction: oldDiscardAction,
}: Props,
) {
return Boolean(
message !== oldMessage ||
!isEqual(saveAction, oldsaveAction) ||
!isEqual(discardAction, oldDiscardAction),
React.useEffect(
() => {
return () => {
if (!frame) return;
frame.removeContextualSaveBar();
};
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[],
);
}

export default compose<Props>(
withContext<Props, WithAppProviderProps, FrameContextType>(
FrameContext.Consumer,
),
withAppProvider(),
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 was never being used

)(ContextualSaveBar);
return null;
});
18 changes: 18 additions & 0 deletions src/components/ContextualSaveBar/tests/ContextualSaveBar.test.tsx
@@ -1,4 +1,5 @@
import React from 'react';
import {mount} from 'enzyme';
import {mountWithAppProvider} from 'test-utilities';
import ContextualSaveBar from '../ContextualSaveBar';
import {FrameContext, createFrameContext} from '../../Frame';
Expand Down Expand Up @@ -86,6 +87,23 @@ describe('<ContextualSaveBar />', () => {
});
expect(mockFrameContext.setContextualSaveBar).toHaveBeenCalledTimes(1);
});

it("doesn't call setContextualSave when frame is not defined", () => {
function fn() {
mount(<ContextualSaveBar {...props} />);
}

expect(fn).not.toThrow();
});

it("doesn't call removeContextualSave when frame is not defined", () => {
function fn() {
const contextualSavebar = mount(<ContextualSaveBar {...props} />);
contextualSavebar.unmount();
}

expect(fn).not.toThrow();
});
});

function noop() {}