[feature] Add mobile navigation menu

[fuller]

What I did

• Create mobile navigation that will apply when the screen is less than 650px
  • Media query value was based on when the content seemed to best wrap
  • The largest changes involve layout shifts and the creation of a MobileLayout and DesktopLayout to render the appropriate containers and styles
  • All behavior of the LeftPanel and DownPanel should have all expected functionality
• Introduction of a Collapsible / Accordion component that expands and focuses on the content area. The content should also be expanded when filtering.
• Increased a11y of the menus by adding href to the anchor tags, this allows them to be keyboard focusable, specifically when tabbing through the interface.

UX note

I increased the font size of filter input from 14px to 1em (equivalent of 16px). Without this it would cause an unwanted page zoom when focused on the input. I've adjusted the appropriate height and close button styles to reflect appropriately.

Future improvements

For a future PR would there be any interest in moving to styled-components, glamorous, jsxstyle, fela, etc? Working with purely inline styles is fairly limiting.

How to test

Deployed the example application with now.sh to:
https://example-pzrtwcexit.now.sh

• Tested with iPhone 7 Plus and it appears to be working as expected
  • Unfortunately, I don't have any other test devices at home. I'll try to get my hands on some Android phones in the office.

[fuller]

@ndelangen @usulpro this should be back up and running now against the monorepo 🎉 . I've also deployed an example here: https://example-pzrtwcexit.now.sh

Please let me know if you have any questions.

[ndelangen]

Do you think the filter should stay visible at all times?
I think it might be better if it was inside the collapse / expand region, so you can filter only when it's expanded..

Is this something you can do? I think we can merge this either way!

[ndelangen]

Thank you so much for taking the time and effort @ajfuller ❤️

[fuller]

@ndelangen that makes sense, I'll make that change and fix the additional lint errors

[fuller]

@ndelangen I tried pulling in the latest master branch but it looks like the Jest changes may be causing some test failures (https://travis-ci.org/storybooks/storybook/builds/222999766)

Any suggestions?

[shilman]

@ajfuller first, thanks so much for the contribution. This is a great feature!

To fix the test issues, I think you need to update from master and resolve conflicts. Your branch is missing later changes regarding lerna bootstrap in package.json among other things. when I updated there were only a few test errors left, and I believe they are related to this change.

# update from master and manually resolve conflicts
npm install
npm run bootstrap
npm test

Hope that helps!

[shilman]

I think React.PropTypes is deprecated. Should be import PropTypes from 'prop-types' across all changes AFAIK?

[fuller]

Thanks @shilman, I'll try to get this rebased against master tomorrow and let you know!

[fuller]

Pulling in all of the project changes since I first started this branch seem to really have screwed everything up. I may just close this and cherry-pick the changes back on top of master.

[codecov]

Codecov Report

Merging #908 into master will decrease coverage by 14.36%.
The diff coverage is 24.59%.

@@             Coverage Diff             @@
##           master     #908       +/-   ##
===========================================
- Coverage   26.72%   12.35%   -14.37%     
===========================================
  Files         192      196        +4     
  Lines        4427     4638      +211     
  Branches      707      735       +28     
===========================================
- Hits         1183      573      -610     
- Misses       3244     3413      +169     
- Partials        0      652      +652

Impacted Files	Coverage Δ
...ui/src/modules/ui/components/left_panel/stories.js	100% <ø> (ø)	⬆️
...ybook-ui/src/modules/ui/components/layout/index.js	100% <100%> (+24.52%)	⬆️
...rc/modules/ui/components/left_panel/text_filter.js	30.35% <15.78%> (-8.86%)	⬇️
...book-ui/src/modules/ui/components/layout/mobile.js	11.36% <16.66%> (ø)
...ook-ui/src/modules/ui/components/layout/desktop.js	27.81% <29.36%> (ø)
...rybook-ui/src/modules/ui/components/collapsible.js	4.34% <5.66%> (ø)
...modules/ui/components/layout/commonLayoutStyles.js	50% <50%> (ø)
...k-ui/src/modules/ui/components/left_panel/index.js	84.61% <77.77%> (-15.39%)	⬇️
packages/addon-info/src/index.js	0% <0%> (-66.67%)	⬇️
...mments/src/manager/components/CommentItem/style.js	0% <0%> (-50%)	⬇️
... and 135 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 62bf99a...7eb1491. Read the comment docs.

[usulpro]

@ajfuller Thank you for your efforts to implement this truly necessary feature!

I really like this and I would like to make a few suggestions:

1. Don't create completely new layout for mobile devices

You're adding the new layout for mobile devices here and switching to it when media query matches (max-width: 650px). In this I see the following disadvantages:

• I tested it with tablet and it switches every time when I change from portrait to landscape mode

• Because of two different layouts, it cause such issue as described here

• In general, I'd prefer to have the only layout both for mobile/tablet and desktop and adjust it via props. At least as far as it possible. It could be easily for us to test and implement features in this case.

What can we do:
There is an option leftPanelOnTop now. At this moment you can only enable it manually. Could you check it out - looks similar to your mobile layout? How do you think could we use it instead? In this case, we just need to pass a prop to layout in order to enable it. And it doesn't break our settings.

2. Don't use react-media

I actually have no objection to media queries but I'd like to offer another approach to handle them.
But at this moment you use <Media query="(max-width: 650px)"> twice. And I guess possibly we may want to use them in other places as well.

What can we do:
What if we handle media queries directly and pass this data to containers so each element would have access to it?
We can add listener and invoke API the similar way as we do it with keyboard shorcuts. Here is an example of it.

How do you think, could we implement this? I'd happy to help in any case. And it' still my thoughts. Maybe someone in @storybooks/team have objections or better ideas for this?

[ndelangen]

I need to rebase this

[ndelangen]

@usulpro @ajfuller Do you think you could find a way to get this in a merga-able state?

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. We do try to do some housekeeping every once in a while so inactive issues will get closed after 90 days. Thanks!

[ndelangen]

I fear that this has become un-mergable with all the changes made lately, I will investigate in adding mobile support to #2101

@ajfuller Please contact me if you want to pair on that feature, I'd be super super happy to!