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

Fix centered addon for IE11 #3735

Merged
merged 2 commits into from
Jun 16, 2018

Conversation

sam-at-work
Copy link

Issue:
#2525

What I did

Fix the style rules for centered add on so that it works correctly in IE11

How to test

Look at centered add on in IE 11. Components will now be correctly centered

@sam-at-work sam-at-work requested a review from kazupon as a code owner June 8, 2018 15:06
@sam-at-work
Copy link
Author

@brianespinosa you might be interested in checking this?

@danielduan
Copy link
Member

danielduan commented Jun 8, 2018

Looks good but I think this breaks some snapshots so those will need to be updated. @sam-at-work

yarn bootstrap, select core, then yarn test and select update

@danielduan danielduan added patch:yes Bugfix & documentation PR that need to be picked to main branch addon: centered labels Jun 8, 2018
@codecov
Copy link

codecov bot commented Jun 11, 2018

Codecov Report

Merging #3735 into master will decrease coverage by 0.27%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3735      +/-   ##
==========================================
- Coverage   41.59%   41.32%   -0.28%     
==========================================
  Files         462      459       -3     
  Lines        5147     5084      -63     
  Branches      866      853      -13     
==========================================
- Hits         2141     2101      -40     
+ Misses       2490     2473      -17     
+ Partials      516      510       -6
Impacted Files Coverage Δ
addons/centered/src/styles.js 0% <ø> (ø) ⬆️
addons/info/src/index.js 76.19% <0%> (-2.98%) ⬇️
app/angular/src/server/angular-cli_utils.js 0% <0%> (ø) ⬆️
.../storyshots-core/src/frameworks/frameworkLoader.js
...s/storyshots-core/src/frameworks/vue/renderTree.js
...s/storyshots-core/src/frameworks/angular/loader.js
addons/storyshots/storyshots-core/.eslintrc.js
addons/storyshots/storyshots-core/src/api/index.js
.../storyshots-core/src/frameworks/require_context.js
...dons/storyshots/storyshots-core/src/test-bodies.js
... and 43 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86f37eb...f0b7885. Read the comment docs.

@sam-at-work
Copy link
Author

@danielduan I've updated the snapshots.
Got an error running yarn test then update:

$ yarn test
yarn run v1.7.0
$ node ./scripts/test.js
? Select which tests to run Update all snapshots (update)
● Validation Error:

  Module <rootDir>/node_modules/react-native/jest/assetFileTransformer.js in the transform option was not found.

  Configuration Documentation:
  https://facebook.github.io/jest/docs/configuration.html

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

but running node node_modules/jest/bin/jest.js -u worked.

@sam-at-work sam-at-work force-pushed the fix-centered-addon-ie11 branch from c582da9 to f0b7885 Compare June 13, 2018 11:16
@danielduan
Copy link
Member

@Hypnosphi I'm not sure why the Danger CI is failing, but this looks good to go.

@igor-dv igor-dv added the bug label Jun 14, 2018
@Hypnosphi Hypnosphi merged commit 12b41f1 into storybookjs:master Jun 16, 2018
@sam-at-work sam-at-work deleted the fix-centered-addon-ie11 branch June 19, 2018 09:42
@Hypnosphi Hypnosphi added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jun 20, 2018
Hypnosphi added a commit that referenced this pull request Jun 20, 2018
lowsky referenced this pull request in lowsky/gh-dashboard-relay Jul 19, 2018
This Pull Request renovates the package group "storybook monorepo".


-   [@&#8203;storybook/react](https://github.com/storybooks/storybook) (`devDependencies`): from `3.4.7` to `3.4.8`
-   [@&#8203;storybook/addons](https://github.com/storybooks/storybook) (`devDependencies`): from `3.4.7` to `3.4.8`
-   [@&#8203;storybook/addon-links](https://github.com/storybooks/storybook) (`devDependencies`): from `3.4.7` to `3.4.8`
-   [@&#8203;storybook/addon-actions](https://github.com/storybooks/storybook) (`devDependencies`): from `3.4.7` to `3.4.8`

# Release Notes
<details>
<summary>storybooks/storybook</summary>

### [`v3.4.8`](https://github.com/storybooks/storybook/blob/master/CHANGELOG.md#&#8203;348)
[Compare Source](storybookjs/storybook@v3.4.7...v3.4.8)
2018-June-21
##### Bug Fixes

-   Fix centered addon for IE11 [#&#8203;3735](`https://github.com/storybooks/storybook/pull/3735`)
-   Display functions as variables not invocations [#&#8203;3761](`https://github.com/storybooks/storybook/pull/3761`)

---


</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: centered bug patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants