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

[ThemeProvider] Remove componentWillReceiveProps #1254

Merged
merged 2 commits into from
Apr 18, 2019

Conversation

AndrewMusgrave
Copy link
Member

WHY are these changes introduced?

Remove deprecated lifecycle methods. This is in preparation for using the new context api.

WHAT is this pull request doing?

Storing context on state, getChildContext will be ran every time state changes and making use of componentDidUpdate.

How to 馃帺

@BPScott BPScott temporarily deployed to polaris-react-pr-1254 March 27, 2019 14:53 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1254 March 27, 2019 14:57 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1254 March 27, 2019 15:23 Inactive
@AndrewMusgrave AndrewMusgrave added the 馃Skip Changelog Causes CI to ignore changelog update check. label Mar 27, 2019
@@ -96,4 +96,37 @@ describe('<ThemeProvider />', () => {
'--top-bar-color': 'rgb(255, 255, 255)',
});
});

it('updates themes', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Additional coverage to bring us to 馃挴

Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

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

馃帺 馃憖 solid 馃帀馃憤 :shipit:

@alex-page added you because I figure you might have/want a bit more context on the current theming stuff. Here's some playground code that updates and combines the different ones from the old PR Andrew linked to (you can checkout tp-rm-cwrp-w-latest-master if you want to tophat in the new storybook, I just couldn't go back to the pre-v5 version 馃槄):

Click to view collapsed playground code
import * as React from 'react';
import {
  Page,
  AppProvider,
  ThemeProvider,
  TopBar,
  Frame,
  Card,
  ActionList,
  Button,
  ChoiceList,
} from '../src';

interface State {}

enum ThemeChoice {
  AppProviderThemed,
  ThemeProviderThemed,
  AppAndThemeProviderThemed,
  AppProviderThemedSubscription,
  ThemeProviderThemedSubscription,
}

export default class Playground extends React.Component<never, State> {
  state = {
    backgroundOne: '#00848E',
    backgroundTwo: '#BADA55',
    selected: [ThemeChoice.AppProviderThemed],
  };

  handleChange = (selected) => {
    this.setState({selected});
  };

  render() {
    const {backgroundOne, backgroundTwo, selected} = this.state;

    const topBarMarkup = (
      <TopBar
        showNavigationToggle
        userMenu={
          <TopBar.UserMenu
            open={false}
            actions={[
              {
                items: [
                  {content: 'Your profile', icon: 'profile'},
                  {content: 'Log out', icon: 'logOut'},
                ],
              },
              {
                items: [
                  {content: 'Shopify help center'},
                  {content: 'Community forums'},
                ],
              },
            ]}
            name="Ellen Ochoa"
            detail="Ochoa Crafts"
            initials="EO"
            onToggle={() => {}}
          />
        }
        searchField={
          <TopBar.SearchField
            onChange={() => {}}
            value=""
            placeholder="Search"
          />
        }
      />
    );

    const themeApp = {
      colors: {
        topBar: {
          background: '#108043',
        },
      },
      logo: {
        width: 130,
        topBarSource:
          'https://cdn.shopify.com/shopify-marketing_assets/static/shopify-full-color-white.svg',
      },
    };

    const themeTheme = {
      colors: {
        topBar: {
          background: '#BADA55',
        },
      },
      logo: {
        width: 130,
        topBarSource:
          'https://cdn.shopify.com/shopify-marketing_assets/static/shopify-full-color-white.svg',
      },
    };

    const themeSubscriptionOne = {
      colors: {
        topBar: {
          background: backgroundOne,
        },
      },
      logo: {
        width: 130,
        topBarSource:
          'https://cdn.shopify.com/shopify-marketing_assets/static/shopify-full-color-white.svg',
      },
    };

    const themeSubscriptionTwo = {
      colors: {
        topBar: {
          background: backgroundTwo,
        },
      },
      logo: {
        width: 130,
        topBarSource:
          'https://cdn.shopify.com/shopify-marketing_assets/static/shopify-full-color-white.svg',
      },
    };

    const choiceList = (
      <ChoiceList
        title="Choose how to theme"
        choices={[
          {
            label: 'App provider themed',
            value: ThemeChoice.AppProviderThemed,
          },
          {
            label: 'Theme provider themed',
            value: ThemeChoice.ThemeProviderThemed,
          },
          {
            label: 'App provider and theme provider themed',
            value: ThemeChoice.AppAndThemeProviderThemed,
            helpText: 'Theme provider theme overrides app provider theme',
          },
          {
            label: 'App provider themed with subscription',
            value: ThemeChoice.AppProviderThemedSubscription,
            renderChildren: (isSelected) => {
              return (
                isSelected && (
                  <Button
                    onClick={() => {
                      this.setState(({backgroundOne}) => ({
                        backgroundOne:
                          backgroundOne === '#50248F' ? '#00848E' : '#50248F',
                      }));
                    }}
                  >
                    Toggle theme
                  </Button>
                )
              );
            },
          },
          {
            label: 'Theme provider themed with subscription',
            value: ThemeChoice.ThemeProviderThemedSubscription,
            renderChildren: (isSelected) => {
              return (
                isSelected && (
                  <Button
                    onClick={() => {
                      this.setState(({backgroundTwo}) => ({
                        backgroundTwo:
                          backgroundTwo === '#50248F' ? '#BADA55' : '#50248F',
                      }));
                    }}
                  >
                    Toggle theme
                  </Button>
                )
              );
            },
          },
        ]}
        selected={selected}
        onChange={this.handleChange}
      />
    );

    const appProviderThemed = selected[0] === ThemeChoice.AppProviderThemed && (
      <AppProvider theme={themeApp}>
        <Frame topBar={topBarMarkup}>
          <Page title="Playground">{choiceList}</Page>
        </Frame>
      </AppProvider>
    );

    const themeProviderThemed = selected[0] ===
      ThemeChoice.ThemeProviderThemed && (
      <AppProvider>
        <ThemeProvider theme={themeTheme}>
          <Frame topBar={topBarMarkup}>
            <Page title="Playground">{choiceList}</Page>
          </Frame>
        </ThemeProvider>
      </AppProvider>
    );

    const appAndThemeProviderThemed = selected[0] ===
      ThemeChoice.AppAndThemeProviderThemed && (
      <AppProvider theme={themeApp}>
        <ThemeProvider theme={themeTheme}>
          <Frame topBar={topBarMarkup}>
            <Page title="Playground">{choiceList}</Page>
          </Frame>
        </ThemeProvider>
      </AppProvider>
    );

    const appProviderThemedSubscription = selected[0] ===
      ThemeChoice.AppProviderThemedSubscription && (
      <AppProvider theme={themeSubscriptionOne}>
        <Frame topBar={topBarMarkup}>
          <Page title="Playground">{choiceList}</Page>
        </Frame>
      </AppProvider>
    );

    const themeProviderThemedSubscription = selected[0] ===
      ThemeChoice.ThemeProviderThemedSubscription && (
      <AppProvider>
        <ThemeProvider theme={themeSubscriptionTwo}>
          <Frame topBar={topBarMarkup}>
            <Page title="Playground">{choiceList}</Page>
          </Frame>
        </ThemeProvider>
      </AppProvider>
    );

    return (
      <React.Fragment>
        {appProviderThemed}
        {themeProviderThemed}
        {appAndThemeProviderThemed}
        {appProviderThemedSubscription}
        {themeProviderThemedSubscription}
      </React.Fragment>
    );
  }
}

Copy link
Member

@alex-page alex-page left a comment

Choose a reason for hiding this comment

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

Thanks @chloerice and great work @AndrewMusgrave.

I am interested why there is a slight delay in changing the search bar colour. I feel like we could look into that in the future though.

@AndrewMusgrave
Copy link
Member Author

I am interested why there is a slight delay in changing the search bar colour. I feel like we could look into that in the future though.

Without pulling this down to check, I'm pretty sure it's our animation that causing the delay.

@AndrewMusgrave
Copy link
Member Author

Thanks for putting all the examples into one @chloerice , it was a great idea 鉂わ笍

@AndrewMusgrave AndrewMusgrave merged commit 55bde16 into version-4.0.0 Apr 18, 2019
@AndrewMusgrave AndrewMusgrave deleted the tp-rm-cwrp branch April 18, 2019 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
馃Skip Changelog Causes CI to ignore changelog update check.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants