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

Feature: Menu with core functionalities #981

Closed
wants to merge 3 commits into from

Conversation

usulpro
Copy link
Member

@usulpro usulpro commented Apr 30, 2017

Issues: #867 #314 ui-#69

step towards: #124 #868 #862

At this moment we can only use keyboard shortcuts and query string to get core functionalities. In some cases, we'd like to do the same via mouse or touch screen. Especially necessary for access from mobile devices

What I did

1. Added menu duplicate keyboard shortcuts to the left (stories) panel

expanded menu

image

This menu is collapsible (default state):

image

This menu has buttons to navigate within stories / storyKinds

image

We'll need it in Fullscreen mode

Removed `⌘` button from the header of left panel

image

It's still available from the core menu

2. Added floating menu when left panel is hidden

Fullscreen mode

image

So we can navigate via mouse/touchscreen or return to the normal view

Hidden left panel

image

Expanse direction

image

Depending on its position menu expands to up or down direction

3. Mantra modules API

  • jumpToKind - new API to jump between storyKinds: modules/api/actions

  • floatingBoxOptions - settings of floating menu displaying. Default:

floatingBoxOptions: { 
      showFloatingBox: true, 
      floatingBoxPosition: boxPositions.BOTTOM_LEFT, 
}

4. Added tests for new components

How to test

  • Open demo page to test usability. Try it with mobile devices.

  • Clone repo

cd packages/storybook-ui
npm link
cd sample_repo
npm link @kadira/storybook-ui
npm start storybook

Try to use new menu to hide/show left/down panels. Go to fullscreen/normal mode. Navigate through the stories.

To change floating menu position set default settings to one of this

  • Run tests
cd storybook
npm run test

All tests should pass

Details for review
  • New react component for menu is here: packages/storybook-ui/src/modules/ui/components/core_menu/

  • It gets access to API via react-komposer here: packages/storybook-ui/src/modules/ui/containers/core_menu.js

  • new jumpToKind API is here: packages/storybook-ui/src/modules/api/actions/api.js

  • all svg images for menu in the webpack bundled library: packages/storybook-ui/src/modules/ui/components/assets/svg_package.js. we can discuss this solution

@codecov
Copy link

codecov bot commented Apr 30, 2017

Codecov Report

Merging #981 into master will increase coverage by 1.47%.
The diff coverage is 47.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #981      +/-   ##
==========================================
+ Coverage   12.61%   14.08%   +1.47%     
==========================================
  Files         192      198       +6     
  Lines        4424     4658     +234     
  Branches      707      733      +26     
==========================================
+ Hits          558      656      +98     
- Misses       3241     3362     +121     
- Partials      625      640      +15
Impacted Files Coverage Δ
packages/storybook-ui/src/modules/ui/index.js 0% <ø> (ø) ⬆️
...book-ui/src/modules/shortcuts/actions/shortcuts.js 5.55% <0%> (-0.7%) ⬇️
packages/storybook-ui/src/libs/key_events.js 22.72% <0%> (-0.53%) ⬇️
...ckages/storybook-ui/src/modules/api/actions/api.js 39.09% <0%> (-10.34%) ⬇️
...k-ui/src/modules/ui/components/left_panel/index.js 100% <100%> (ø) ⬆️
...es/storybook-ui/src/modules/ui/components/theme.js 100% <100%> (ø) ⬆️
...ybook-ui/src/modules/ui/components/layout/index.js 74.07% <100%> (+0.48%) ⬆️
...-ui/src/modules/ui/components/left_panel/header.js 25% <100%> (-0.93%) ⬇️
...s/storybook-ui/src/modules/ui/containers/layout.js 5.55% <25%> (-6.95%) ⬇️
.../modules/ui/components/core_menu/floating_block.js 25% <31.81%> (ø)
... and 15 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 7910ae6...1d88caa. Read the comment docs.

@danielduan
Copy link
Member

I would love to see this merged sooner rather than later. Can you rebase this?

@@ -0,0 +1,175 @@
/* eslint-disable */
(function webpackUniversalModuleDefinition(root, factory) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not following why we're committing a webpack generated module into the repo with svgs in base64 string format.

I think for maintainability sake, we should include the original svgs in the repo and generate them during runtime.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you @danielduan
It's a legacy solution from PR for Storybook 2.0 :)
I think we can switch to the "normal" way to import svg.
We'll need to change some loaders in webpack config, for that. Actually, I think we need to do it very carefully!

@ndelangen ndelangen added this to the v3.1.0 milestone May 26, 2017
@ndelangen ndelangen removed this from the v3.2.0 milestone Jul 7, 2017
@danielduan
Copy link
Member

Closing this because there hasn't been any activity here for a while. Feel free to reopen when this is ready for discussion again.

@danielduan danielduan closed this Nov 14, 2017
@bjankord
Copy link

bjankord commented Dec 4, 2017

I'm really interested in this PR, the ability to toggle all the panels makes viewing stories a lot easier on mobile devices. I'm currently writing scripts in manager-head.html to toggle the panels but its very much hacked together, I'd much prefer a first class storybook solution to handle this. @usulpro, you mentioned needing to change some loaders in webpack config. Is that work you might be interested in picking up?

@danielduan
Copy link
Member

danielduan commented Dec 4, 2017

@bjankord he's been unresponsive for a while now. The best thing you can do is pick up where he left off and try working on it.

You can make a branch off his and open a PR when you're ready.

Copy link

nx-cloud bot commented Apr 4, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 1d88caa. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

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.

5 participants