React Native collapsible OnDeviceUI navigation #1544

rmevans9 · 2017-07-30T03:02:10Z

Issue: N/A

What I did

Modified the styles for the OnDeviceMenu to make the menu something you can show/hide instead of always taking up the space.

NOTES

Right now it is not "fancy" meaning no animation or anything. I plan on adding that likely in a commit within the next 24 hours.
Instead of un-mounting the menu like I originally planned I had to simply move it off screen as the mount/unmount causes issues with resetting which story you are looking at. This could probably be solved so you could unmount however since I plan on animating it I will need it to sit off screen anyway... so lets call it "planning for the future"
I found that a method call in the StoryListView was incorrect when cleaning up listeners. I fixed that.
Right now the trigger for opening/closing the menu is just text. We probably will want something better.

How to test

Run storybook with OnDeviceUI and see it.

codecov · 2017-07-30T10:27:00Z

Codecov Report

Merging #1544 into master will decrease coverage by 0.08%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1544      +/-   ##
==========================================
- Coverage   20.53%   20.45%   -0.09%     
==========================================
  Files         241      241              
  Lines        5220     5242      +22     
  Branches      650      652       +2     
==========================================
  Hits         1072     1072              
+ Misses       3666     3662       -4     
- Partials      482      508      +26

Impacted Files	Coverage Δ
...tive/src/preview/components/StoryListView/style.js	`0% <ø> (ø)`	⬆️
...tive/src/preview/components/StoryListView/index.js	`0% <0%> (ø)`	⬆️
...-native/src/preview/components/OnDeviceUI/style.js	`0% <0%> (ø)`	⬆️
...-native/src/preview/components/OnDeviceUI/index.js	`0% <0%> (ø)`	⬆️
lib/ui/src/libs/key_events.js	`23.25% <0%> (ø)`	⬆️
app/react/src/client/preview/error_display.js	`0% <0%> (ø)`	⬆️
lib/ui/src/modules/ui/containers/left_panel.js	`25% <0%> (ø)`	⬆️
addons/knobs/src/components/types/Color.js	`8% <0%> (ø)`	⬆️
lib/ui/src/modules/ui/libs/filters.js	`53.33% <0%> (ø)`	⬆️
addons/knobs/src/KnobManager.js	`35.41% <0%> (ø)`	⬆️
... and 14 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 73fd1dc...bcb4db8. Read the comment docs.

rmevans9 · 2017-07-30T10:29:44Z

I have added an animation to the open/close of the menu to make it feel better. Now its just the styles of the menu and what the open/close button should be.

I am thinking a "hamburger" menu icon but wanted to see if there were any other opinions?

rmevans9 · 2017-07-30T10:41:50Z

I am considering adding the ability to tap outside of the menu to close it as well.

…ouchable to close

rmevans9 · 2017-07-30T11:08:00Z

Added an overlay over the preview window that fades in with the menu opening. It is touchable so that you can tap outside of the menu to close it like you would expect to be able to do in a drawer menu

rmevans9 · 2017-07-30T11:22:30Z

I am not completely sold on the location of the buttons yet.

Images came from:
https://www.iconfinder.com/icons/134226/arrow_back_left_icon#size=128
https://www.iconfinder.com/icons/134216/hamburger_lines_menu_icon

shilman · 2017-07-30T15:43:28Z

From our slack chat:

atticoos

I'm so excited for this one 😃 this is going to make the experience on devices so nice. I have some very minor code style feedback as I was skimming through, and otherwise just one clarifying question. Great job!

atticoos · 2017-07-30T17:26:59Z

app/react-native/src/preview/components/OnDeviceUI/index.js

-  const { stories, events, url } = props;
+export default class OnDeviceUI extends Component {
+  constructor(props, ...args) {
+    super(props, ...args);


Super nitty feedback - but since props isn't used, we could just spread all the arguments

constructor(...props) { super(...props) ... }

Good catch. I had props in there before and just forgot to clean this up. I'll get it updated.

atticoos · 2017-07-30T17:28:29Z

app/react-native/src/preview/components/OnDeviceUI/index.js

+        {isMenuOpen &&
+          <TouchableWithoutFeedback onPress={this.handleToggleMenu}>
+            <Animated.View style={overlayStyles} />
+          </TouchableWithoutFeedback>}


dropping the trailing } onto another line would make this more legible -- thought Animated.View was part of this condition block 😃

Prettier is not a fan of that. I typically would have it on the next line but prettier complained at me. I'll look at options because I agree that this isn't as readable as it could be.

Oh, if prettier did this, then nothing to see here!

Resolved by way of removing this code all together 😄

atticoos · 2017-07-30T17:36:34Z

app/react-native/src/preview/components/OnDeviceUI/index.js

+  }
+
+  render() {
+    const { stories, events, url } = this.props;


Do we need to subscribe for stories being added? Or will added stories become updated in props.stories and lead to a re-render?

Update: Deferred to #1547

Honestly not sure. The subscription setup was actually setup by someone else. This PR is just changing the UI to be more giving on space available to the preview section. I can work on cleaning this up in a follow up PR

The original component received added stories through event emissions -- I think we may want to subscribe to those here too, or otherwise the sidebar may not update as stories become added. cc @shilman or @ndelangen -- do Preview components receive additional stories on props.stories over time? Or are subscriptions needed

@ajwhite right now in examples/react-native-vanilla it appears the stories don't update in the browser OR in the OnDeviceUI when I modify the story code. I'm not sure if this is a bug in my setup or whether something is broken in @storybook/react-native or just the example...

Okay - sounds like something we can come back to and keep the scope of this PR narrow 👍

Opened #1547, can probably tackle once I'm back in Boston on Tuesday.

atticoos · 2017-07-30T17:38:03Z

app/react-native/src/preview/components/OnDeviceUI/style.js

+    color: 'rgba(0, 0, 0, 0.5)',
+  },
+  overlayContainer: {
+    position: 'absolute',


For consistency, you can use StyleSheet.absoluteFillObject like used below in the preview style 😃

Good catch. I meant to switch them all to be the same but forgot. Will get this updated

shilman · 2017-07-30T20:23:38Z

@rmevans9 This is looking great! I have some issues with the current design though. If you look at the old behavior, it's useful to be able to click through different UI states quickly in the old UI:

This ability is greatly diminished in the new version, because the story contents are both obscured by the menu and also dimmed.

My suggestion is that the menu's slideout behavior pushes the story contents to the side rather than obscures it. This enables the old behavior, which is great for landscape phones and all tablets (I think, I haven't actually tried it on a tablet.)

rmevans9 · 2017-07-30T20:41:02Z

No problem, I can get that fixed up here in a couple of hours. If we shift the content part of the right side of the preview window will go off screen, would that be an issue you think?

rmevans9 · 2017-07-30T20:43:55Z

On the other hand I could adjust the size of the preview window, some people components may not handle that well though.

shilman · 2017-07-30T20:44:30Z

@rmevans9 Is it possible to shrink the preview window instead? The current behavior is one in which the preview screen is narrower by the width of the menu, which means that if the component is centered, it works pretty well on wider screens. I'm not sure what is possible on RN.

rmevans9 · 2017-07-30T20:58:57Z

Yeah, it might just be that some users components act oddly when shrinking/expanding but we can cross that bridge when it happens. I'll get these changes in soon

rmevans9 · 2017-07-30T22:20:19Z

@shilman per slack convo - updated design is in based upon your request. Let me know if there is anything else you see that you want changed!

shilman

LGTM! 👍

And in this case G = GREAT

shilman · 2017-07-30T23:28:15Z

@rmevans9 i'm gonna merge this into release/3.2 so we can get an alpha out without compromising the release

rmevans9 added 2 commits July 29, 2017 22:57

Tweak the UI to make the menu not so intrusive

9cb655a

Add animations to the open/close of the menu

feb86f3

Add a overlay over the preview window when the menu is open that is t…

2c095c2

…ouchable to close

Add images instead of text for buttons.

80b9015

Style the buttons a little better

79deb67

Updates

f425d2f

atticoos reviewed Jul 30, 2017

View reviewed changes

Android fixes + design change

d6c2128

Hide the statusbar for now to avoid an ackward situation in landscape

bcb4db8

shilman approved these changes Jul 30, 2017

View reviewed changes

shilman changed the base branch from master to release/3.2 July 30, 2017 23:28

shilman merged commit 2bf095c into storybookjs:release/3.2 Jul 30, 2017

shilman changed the title ~~[WIP] OnDeviceUI menu tweaks~~ React Native collapsible OnDeviceUI navigation Jul 31, 2017

atticoos added react-native feature request labels Jul 31, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

React Native collapsible OnDeviceUI navigation #1544

React Native collapsible OnDeviceUI navigation #1544

rmevans9 commented Jul 30, 2017

codecov bot commented Jul 30, 2017 •

edited

Loading

rmevans9 commented Jul 30, 2017

rmevans9 commented Jul 30, 2017

rmevans9 commented Jul 30, 2017

rmevans9 commented Jul 30, 2017 •

edited

Loading

shilman commented Jul 30, 2017

atticoos left a comment

atticoos Jul 30, 2017

rmevans9 Jul 30, 2017

rmevans9 Jul 30, 2017

atticoos Jul 30, 2017

rmevans9 Jul 30, 2017

atticoos Jul 30, 2017

rmevans9 Jul 30, 2017

atticoos Jul 30, 2017 •

edited

Loading

rmevans9 Jul 30, 2017

atticoos Jul 30, 2017

shilman Jul 30, 2017

atticoos Jul 30, 2017

atticoos Jul 30, 2017

rmevans9 Jul 30, 2017

rmevans9 Jul 30, 2017

shilman commented Jul 30, 2017 •

edited

Loading

rmevans9 commented Jul 30, 2017

rmevans9 commented Jul 30, 2017

shilman commented Jul 30, 2017

rmevans9 commented Jul 30, 2017

rmevans9 commented Jul 30, 2017

shilman left a comment

shilman commented Jul 30, 2017

React Native collapsible OnDeviceUI navigation #1544

React Native collapsible OnDeviceUI navigation #1544

Conversation

rmevans9 commented Jul 30, 2017

What I did

NOTES

How to test

codecov bot commented Jul 30, 2017 • edited Loading

Codecov Report

rmevans9 commented Jul 30, 2017

rmevans9 commented Jul 30, 2017

rmevans9 commented Jul 30, 2017

rmevans9 commented Jul 30, 2017 • edited Loading

shilman commented Jul 30, 2017

atticoos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atticoos Jul 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shilman commented Jul 30, 2017 • edited Loading

rmevans9 commented Jul 30, 2017

rmevans9 commented Jul 30, 2017

shilman commented Jul 30, 2017

rmevans9 commented Jul 30, 2017

rmevans9 commented Jul 30, 2017

shilman left a comment

Choose a reason for hiding this comment

shilman commented Jul 30, 2017

codecov bot commented Jul 30, 2017 •

edited

Loading

rmevans9 commented Jul 30, 2017 •

edited

Loading

atticoos Jul 30, 2017 •

edited

Loading

shilman commented Jul 30, 2017 •

edited

Loading