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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update storybook to the latest version #663

Merged
merged 14 commits into from
Jan 3, 2019
Merged

Update storybook to the latest version #663

merged 14 commits into from
Jan 3, 2019

Conversation

fgameiro
Copy link
Contributor

Closes Issues

Code review checklist

Reviewer 1

  • Functions properly
  • Good readability, clarity and overall quality
  • Has tests
  • Has docs

Reviewer 2

  • Functions properly
  • Good readability, clarity and overall quality
  • Has tests
  • Has docs

…iles

affects: @crave/farmblocks-alert, @crave/farmblocks-amount-selectors, @crave/farmblocks-button,
@crave/farmblocks-card, @crave/farmblocks-carousel, @crave/farmblocks-dropdown,
@crave/farmblocks-empty-state, @crave/farmblocks-filter-popover, @crave/farmblocks-footer,
@crave/farmblocks-form-wrapper, @crave/farmblocks-hoc-disabled-tooltip, @crave/farmblocks-hoc-input,
@crave/farmblocks-hoc-validation-messages, @crave/farmblocks-hoc-withtooltip,
@crave/farmblocks-image, @crave/farmblocks-input-checkbox, @crave/farmblocks-input-currency,
@crave/farmblocks-input-radio, @crave/farmblocks-input-select, @crave/farmblocks-input-text,
@crave/farmblocks-label, @crave/farmblocks-link, @crave/farmblocks-map-balloon,
@crave/farmblocks-more-info, @crave/farmblocks-popover, @crave/farmblocks-dev-scaffold,
@crave/farmblocks-search-field, @crave/farmblocks-stepper, @crave/farmblocks-table,
@crave/farmblocks-tags, @crave/farmblocks-text, @crave/farmblocks-tooltip

Replaced by the global decorator withInfo to the same effect
@fgameiro fgameiro changed the title [WIP] Update storybook to the latest version Update storybook to the latest version Dec 27, 2018
transform: rotate(5deg);
`;

const ExtendedAlert = props => <StyledAlert {...props} />;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do know why the previous version won't work? I wonder if this is the best approach to handle this, because the styled(Alert) should produce a valid component 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I found this to be strange too.
Actually this is just a workaround to avoid this warning:
screen shot 2018-12-27 at 09 45 13

According to this (storybookjs/storybook#4817) it seems that storybook can't access the components' propTypes through the styled wrapper, which raises the warning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ouch...

@alcferreira
Copy link
Contributor

alcferreira commented Dec 27, 2018

Time spent on yarn test -u has increased considerably:

branch storybook master %
188s 74s 154.0540541
221s 64s 245.3125
171s 63s 171.4285714

Can we do something to improve that?

Update: We may want to follow up this issue storybookjs/storybook#4996

package.json Outdated Show resolved Hide resolved
@fgameiro fgameiro changed the title Update storybook to the latest version [WIP] Update storybook to the latest version Dec 28, 2018
affects: @crave/farmblocks-alert, @crave/farmblocks-amount-selectors, @crave/farmblocks-button,
@crave/farmblocks-card, @crave/farmblocks-carousel, @crave/farmblocks-dropdown,
@crave/farmblocks-empty-state, @crave/farmblocks-filter-popover, @crave/farmblocks-footer,
@crave/farmblocks-form-wrapper, @crave/farmblocks-hoc-disabled-tooltip, @crave/farmblocks-hoc-input,
@crave/farmblocks-hoc-validation-messages, @crave/farmblocks-hoc-withtooltip,
@crave/farmblocks-image, @crave/farmblocks-input-checkbox, @crave/farmblocks-input-currency,
@crave/farmblocks-input-radio, @crave/farmblocks-input-select, @crave/farmblocks-input-text,
@crave/farmblocks-link, @crave/farmblocks-map-balloon, @crave/farmblocks-more-info,
@crave/farmblocks-popover, @crave/farmblocks-search-field, @crave/farmblocks-stepper,
@crave/farmblocks-table, @crave/farmblocks-tags, @crave/farmblocks-text, @crave/farmblocks-tooltip
affects: @crave/farmblocks-map-balloon, @crave/farmblocks-popover, @crave/farmblocks-search-field
…onents in stories

affects: @crave/farmblocks-alert, @crave/farmblocks-dropdown, @crave/farmblocks-input-select,
@crave/farmblocks-map-balloon, @crave/farmblocks-search-field
We used to mock the withInfo decorator to prevent snapshots from containing css and html from 'withInfo' addon, but this doesn't work anymore. So we are now preventing the decorator to be added if we are in a test environment.
… config rules

affects: @crave/farmblocks-alert, @crave/farmblocks-amount-selectors, @crave/farmblocks-button,
@crave/farmblocks-card, @crave/farmblocks-carousel, @crave/farmblocks-dropdown,
@crave/farmblocks-empty-state, @crave/farmblocks-filter-popover, @crave/farmblocks-footer,
@crave/farmblocks-form-wrapper, @crave/farmblocks-hoc-disabled-tooltip, @crave/farmblocks-hoc-input,
@crave/farmblocks-hoc-validation-messages, @crave/farmblocks-hoc-withtooltip,
@crave/farmblocks-image, @crave/farmblocks-input-checkbox, @crave/farmblocks-input-currency,
@crave/farmblocks-input-radio, @crave/farmblocks-input-select, @crave/farmblocks-input-text,
@crave/farmblocks-label, @crave/farmblocks-link, @crave/farmblocks-map-balloon,
@crave/farmblocks-more-info, @crave/farmblocks-popover, @crave/farmblocks-dev-scaffold,
@crave/farmblocks-search-field, @crave/farmblocks-stepper, @crave/farmblocks-table,
@crave/farmblocks-tags, @crave/farmblocks-text, @crave/farmblocks-theme, @crave/farmblocks-tooltip

As we use babel to build each package independently, we must allow them to access our babel config
file
located at the root directory of Farmblocks. This commit adds the '--root-mode upward' flag
to each
build command to allow this.
…pdated build command

affects: @crave/farmblocks-dev-scaffold
@fgameiro fgameiro changed the title [WIP] Update storybook to the latest version Update storybook to the latest version Jan 2, 2019
affects: @crave/farmblocks-card
@fgameiro
Copy link
Contributor Author

fgameiro commented Jan 2, 2019

@alcferreira please update your local branch :) I have just rebased it fixing the cardTypes exports

affects: @crave/farmblocks-dev-scaffold
Copy link
Contributor

@alcferreira alcferreira left a comment

Choose a reason for hiding this comment

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

LGTM 🏅

@alcferreira alcferreira merged commit ddf53a9 into master Jan 3, 2019
@alcferreira alcferreira deleted the storybook branch January 3, 2019 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants