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

[onDeviceUI] Add ability to render addons in React Native #3903

Merged
merged 96 commits into from
Oct 11, 2018

Conversation

Gongreg
Copy link
Member

@Gongreg Gongreg commented Jul 20, 2018

Issue

Currently the only way to use addons in React Native is to display them in browser (that means you need to have a server running). This PR allows users to create addons which work in RN app itself.

You can test out the new functionality here! https://expo.io/@gongreg/crna-kitchen-sink.

40x

This is the main PRs for this functionality. Relevant PRs:

#4326
#4327
#4328
#4340

Changes

  • Updated the UI.

    • Moved top bar to the bottom, added border to make it more distinct.
    • Bar now has three buttons: Navigator, preview, addons. Clicking on either navigator or addons makes the preview minimized.
    • You can swipe on the bar to select different panels.
    • You can press on minimized preview to maximize it.
    • Added a button on the right to hide the bar completely.
  • onDeviceUI is now true by default.

You can disable it by passing onDeviceUI as false.

  • Two new options for initial UI display

    • isUIOpen - display bottom bar initially (default - true).
    • tabOpen - which panel to display initially (navigator(-1), preview(0) or addons(1) (default is 0).
  • Safe area view hack (for iphone x) removed.

Now it uses SafeAreaView component. It requires RN 0.50.

  • Reduced amount of rerenders happening when using onDeviceUI.

Before using onDeviceUI made two rerenders on story change events.

  • Channel is created immediately.

Before it was impossible to use addons channel if you didn't render storybookUI component. Now the channel is created as soon as you call getStorybookUI.

Testing

I've updated CRNA example to support on device addons.
I've also published expo example: https://expo.io/@gongreg/crna-kitchen-sink

Part of other PR's

  • Addon registration

cli now creates rn-addons.js file and imports it in entry storybook file.

Current storybook architecture works in two parts. Manager and preview. Adding addons to onDeviceUI removes this separation.

This separation was necessary to support emitting events inside decorators.
Without it now we get warning that renders are not "pure" (they are calling setState while setState is already happening).

By using async channel we avoid this issue since events are emitted only after render has finished.

  • Handles websocket error without crashing.

If storybook tries to connect to non existing storybook server it will not throw red screen anymore.

  • Created new on device addons

    • Backgrounds
    • Knobs
    • Notes
  • Unified template for Storybook installation in React Native.

Both CRNA and RN Vanilla will use same template when installing storybook.

@storybook-safe-bot
Copy link
Contributor

storybook-safe-bot commented Jul 20, 2018

Fails
🚫

PR is marked with "do not merge" label.

Generated by 🚫 dangerJS

@orta
Copy link
Member

orta commented Jul 20, 2018

This is a very clever idea 👍

@@ -1,3 +1,4 @@
import { AddonStore } from '@storybook/addons';
Copy link
Member

Choose a reason for hiding this comment

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

Web part of storybook uses addons.js file. Maybe we should have rn-addons.js file for RN part? Currently I just register the addons next to the stories.

I think having addon.js for RN will consolidate the API.

@@ -3,12 +3,28 @@ import React, { Component } from 'react';
import { getStorybookUI, configure } from '@storybook/react-native';
import { setOptions } from '@storybook/addon-options';

// TODO temp disable till I find a way to fix usage addon
console.disableYellowBox = true;
Copy link
Member

Choose a reason for hiding this comment

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

WHOA 🙀

Copy link
Member Author

Choose a reason for hiding this comment

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

Disabled it in case people wanted to try things out. The usage addon has an issue. It tries to update during a render and thas is a warning. I will fix it :)

isUIOpen - whether to initially show top bar - default true.
isStoryMenuOpen - should show story menu open by default - default false.
@codecov
Copy link

codecov bot commented Aug 2, 2018

Codecov Report

❗ No coverage uploaded for pull request base (on-device-ui-all-prs@6db1f75). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@                   Coverage Diff                   @@
##             on-device-ui-all-prs    #3903   +/-   ##
=======================================================
  Coverage                        ?   36.85%           
=======================================================
  Files                           ?      538           
  Lines                           ?     6441           
  Branches                        ?      839           
=======================================================
  Hits                            ?     2374           
  Misses                          ?     3645           
  Partials                        ?      422
Impacted Files Coverage Δ
...-native/src/preview/components/OnDeviceUI/style.js 0% <ø> (ø)
app/react-native/src/bin/storybook-start.js 0% <0%> (ø)
...rc/preview/components/OnDeviceUI/addons/wrapper.js 0% <0%> (ø)
...ve/src/preview/components/OnDeviceUI/tabs/panel.js 0% <0%> (ø)
...e/src/preview/components/OnDeviceUI/tabs/consts.js 0% <0%> (ø)
...e/src/preview/components/OnDeviceUI/addons/list.js 0% <0%> (ø)
...-native/src/preview/components/OnDeviceUI/index.js 0% <0%> (ø)
...ve/src/preview/components/OnDeviceUI/addons/tab.js 0% <0%> (ø)
...tive/src/preview/components/StoryListView/index.js 0% <0%> (ø)
.../src/preview/components/OnDeviceUI/addons/index.js 0% <0%> (ø)
... and 4 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 6db1f75...18fccca. Read the comment docs.

We cannot use modal since addons expect to be rendered all the time.
So now we display the wrapper as position absolute and make it 0 0 size when it is not visible.
@Gongreg Gongreg changed the title Add ability to render addons in React Native [onDeviceUI] Add ability to render addons in React Native Oct 9, 2018
@Gongreg Gongreg changed the base branch from master to on-device-ui-all-prs October 9, 2018 08:00

## On device addons
On device addons are addons that are displayed in your app in addons panel. You register them in a file called
`rn-addons.js` and you also have to import `rn-addons.js` file somewhere in your entry file (it is done by default
Copy link
Member Author

Choose a reason for hiding this comment

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

Needs better usage explanation

app, they are also rerendered on every change.

## Writing the app addons
On device addons use same addon store and api as web addons. The only difference in api is that you don't have api prop
Copy link
Member Author

Choose a reason for hiding this comment

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

api prop in quotes

app/react-native/docs/manual-setup.md Show resolved Hide resolved


* ### Web addons
There are addons that work on React Native but don't have on device implementation yet.
Copy link
Member Author

Choose a reason for hiding this comment

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

And some addons simply make sense only on the web.

# Storybook server
The default usage of React native storybook till version 5 involved starting storybook server.

The reason it has changed is that in most cases it is not really necessary.
Copy link
Member Author

Choose a reason for hiding this comment

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

Explain more here.

if (Object.keys(panels).length === 0) {
return (
<View style={[style.flex, style.center]}>
<Text style={style.text}>No on device addons loaded</Text>
Copy link
Member

Choose a reason for hiding this comment

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

onDevice

import style from '../style';

export default class Addons extends PureComponent {
static propTypes = {
Copy link
Member

Choose a reason for hiding this comment

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

Some of the components are having props as a static member, while others are defined outside 🤔

style={style.hideButton}
hitSlop={{ top: 5, left: 5, bottom: 5, right: 5 }}
>
<Text style={[style.hideButtonText]}>o</Text>
Copy link
Member

Choose a reason for hiding this comment

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

o

🤣 I am not a fan of this. maybe at least some other ascii? ☰ 🡙 ⥯

(Or swiping down 🤐)

}

Panel.propTypes = {
// eslint-disable-next-line react/forbid-prop-types
Copy link
Member

Choose a reason for hiding this comment

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

What is wrong with the prop types here?

app/react-native/src/preview/index.js Show resolved Hide resolved

This step is done automatically when you install Storybook for the first time and also described in [Manual Setup](https://github.com/storybooks/storybook/blob/master/app/react-native/docs/manual-setup.md)

## Compactibility
Copy link
Member Author

Choose a reason for hiding this comment

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

compatibility

## Performance of on device addons
Because on device addons are inside the app, they are also rerendered on every change. This can reduce performance a lot.

## Writing the app addons
Copy link
Member Author

Choose a reason for hiding this comment

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

on device

app/react-native/docs/addons.md Show resolved Hide resolved
}
};

onLayout = ({ nativeEvent }) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

onLayoutHandler

app/react-native/src/preview/index.js Show resolved Hide resolved
import style from '../style';

export default class Tab extends PureComponent {
onPress = () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Handler

Updated readme,
Renamed handlers.
@Gongreg
Copy link
Member Author

Gongreg commented Oct 10, 2018

This can move forward! @ndelangen, @igor-dv.

@ndelangen ndelangen merged commit d32952d into on-device-ui-all-prs Oct 11, 2018
@slorber
Copy link
Contributor

slorber commented Nov 19, 2018

hey,

This works great, but just wondering if it wouldn't be possible to attach plugins to a chrome extension? The app can already connect to chrome debugger so maybe we could also support showing plugin interfaces like knobs here without needing a server?

@Gongreg
Copy link
Member Author

Gongreg commented Nov 22, 2018

Hey @slorber, I don't have enough knowledge with debugger to comment on that one. But if you have any interest in trying to do that you are definitely welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants