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

Angular: Fix issue if BrowserAnimationsModule is imported #20709

Merged

Conversation

valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Jan 20, 2023

Issue: N/A

If BrowserAnimationModule was used, the Story was not usable because BrowserModule is in place and using both modules in parallel is not allowed. Therefore we have to tell Angular while boostrapApplication to spin up an environment, where Animations are enabled.

What I did

How to test

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@valentinpalkovic valentinpalkovic force-pushed the valentin/fix-angular-browser-animation-use-case branch 4 times, most recently from 27650c1 to dd82575 Compare January 20, 2023 13:01
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

@valentinpalkovic This looks good, but can you please explain more about how to test it? If I tried your example on a previous SB7 beta, would it fail? Thanks in advance for the clarification!

@valentinpalkovic
Copy link
Contributor Author

If you try to use the BrowserAnimationModule in the imports section of moduleMetadata, it will break in previous versions. For example here: https://github.com/storybookjs/storybook/pull/20709/files#diff-eab273740d3db246f7c17bb91c2f9750cb5b9de9fd4da6e8e2e238bc39d1591bR18

@valentinpalkovic
Copy link
Contributor Author

@shilman Just copy over the Stories and the Dummy components and it should fail in previous betas.

Copy link
Contributor

@sheriffMoose sheriffMoose left a comment

Choose a reason for hiding this comment

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

Amazing work @valentinpalkovic , I just verified the changes on my end. Good catch 👍

@valentinpalkovic valentinpalkovic force-pushed the valentin/fix-angular-browser-animation-use-case branch from dd82575 to c550dbb Compare January 24, 2023 10:34
@valentinpalkovic valentinpalkovic merged commit 18ee5c7 into next Jan 24, 2023
@valentinpalkovic valentinpalkovic deleted the valentin/fix-angular-browser-animation-use-case branch January 24, 2023 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants