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

feat: Reintroduce the backgrounds addon #229

Merged

Conversation

lauriharpf
Copy link
Contributor

@lauriharpf lauriharpf commented Jul 16, 2021

Issue: #200 (part of the scope of that issue)

What I did

Reintroduced the backgrounds addon as a part of completing #200. Includes both storiesOf and CSF examples of usage.

How to test

Run the React Native examples under examples/native on the iOS and Android emulators. Experiment with the added examples:

  • Background StoriesOf: Not sure if we want to keep this, given that storiesOf is apparently going away? However, it can be used to demo that this works with existing stories that use the storiesOf format.
  • Background CSF: Same example in the CSF format.

Also check the modified "Backgrounds not configured" text (e.g. open button promise > finally > Addons > Backgrounds).

  • Does this need a new example in examples/native? yes, added both CSF & StoriesOf examples
  • Does this need an update to the documentation? yes; did a minor update to the @storybook/addon-ondevice-backgrounds README, but we should have much better docs & also confirm/add support for all of the web examples

Screen recording of functionality

background.mp4

Known issue: Reloading on iOS does not work

Edit: Never mind, did some changes in 73e783a to fix this.
Reloading the app (e.g. via the emulator or due to making changes) causes the controls in "Addons => Backgrounds" to disappear on iOS. It works correctly on Android. Don't know what is causing this. Any help in fixing is appreciated. See recording below:

bug.mov

…bookjs#200 . Includes both storiesOf and CSF examples of usage.

NOTE: Reloading the app (e.g. via the emulator or due to making changes) causes the controls in "Addons => Backgrounds" to disappear on iOS. It works correctly on Android. Don't know what is causing this.
@lauriharpf lauriharpf requested a review from dannyhw July 16, 2021 11:06
…iOS causing the "Backgrounds" section in addons to render nothing. Fixed this by switched to using the same approach that ondevice-controls seems to use.
@dannyhw
Copy link
Member

dannyhw commented Jul 16, 2021

Can you add an example of the parameter/decorator to the preview.js file? That would apply a global decorator and parameter.

Not entirely sure how necessary the decorator is but it would be something like this

examples/native/.storybook/preview.js

import React from 'react';
import { View, StyleSheet } from 'react-native';
import { withBackgrounds } from '@storybook/addon-ondevice-backgrounds';

export const decorators = [
  (StoryFn) => (
    <View style={styles.container}>
      <StoryFn />
    </View>
  ),
  withBackgrounds,
];
export const parameters = {
  my_param: 'anything',
  backgrounds: [
    { name: 'warm', value: 'hotpink', default: true },
    { name: 'cool', value: 'deepskyblue' },
  ],
};

const styles = StyleSheet.create({
  container: { padding: 8 },
});

- Add dependency to @storybook/addon-ondevice-backgrounds in the examples/native package.json
- Add the withBackgrounds decorator to all stories in examples/native via preview.js
@lauriharpf
Copy link
Contributor Author

Can you add an example of the parameter/decorator to the preview.js file? That would apply a global decorator and parameter.

Thanks, did that 👍 . The only downside of having it globally is that the "backgrounds not configured" functionality (shown in the screen recording at ~0:30) is not visible in examples/native anymore.

@dannyhw
Copy link
Member

dannyhw commented Jul 16, 2021

Can you add an example of the parameter/decorator to the preview.js file? That would apply a global decorator and parameter.

Thanks, did that 👍 . The only downside of having it globally is that the "backgrounds not configured" functionality (shown in the screen recording at ~0:30) is not visible in examples/native anymore.

@lauriharpf ah ok, mostly I just wanted to make sure that decorators/params functionality was working. I think there might be a way to make that message come up though. Perhaps if you replace the backgrounds param with an empty list on one of the stories, not sure though. Either way I think the normal use case is to have this set globally so it's nice to have this example.

Copy link
Member

@dannyhw dannyhw left a comment

Choose a reason for hiding this comment

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

It's working great, impressive work! 🎉

@dannyhw dannyhw merged commit a1c0efd into storybookjs:next-6.0 Jul 16, 2021
@lauriharpf lauriharpf deleted the feat/reintroduce-backgrounds-addon branch July 17, 2021 07:04
dannyhw added a commit to raychanks/react-native that referenced this pull request Feb 7, 2022
* feat: Reintroduce the backgrounds addon as a part of completing storybookjs#200 . Includes both storiesOf and CSF examples of usage.

NOTE: Reloading the app (e.g. via the emulator or due to making changes) causes the controls in "Addons => Backgrounds" to disappear on iOS. It works correctly on Android. Don't know what is causing this.

* docs: Updated the README documentation for the backgrounds addon

* feat: Reintroduce the backgrounds addon. Fix issue with reloading on iOS causing the "Backgrounds" section in addons to render nothing. Fixed this by switched to using the same approach that ondevice-controls seems to use.

* Code review comments:

- Add dependency to @storybook/addon-ondevice-backgrounds in the examples/native package.json
- Add the withBackgrounds decorator to all stories in examples/native via preview.js

* fix: text now shows without dimenions.get(window)

* fix: update code sample

Co-authored-by: Daniel Williams <[email protected]>
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.

None yet

2 participants