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

enableShortcuts not working in v5 #6569

Closed
dogoku opened this issue Apr 20, 2019 · 43 comments · Fixed by #21291
Closed

enableShortcuts not working in v5 #6569

dogoku opened this issue Apr 20, 2019 · 43 comments · Fixed by #21291

Comments

@dogoku
Copy link

dogoku commented Apr 20, 2019

Describe the bug
The enableShortcuts and isFullscreen options are not working as expected.

To Reproduce
Add this to config.js (skipping boilerplate)

addParameters({
  options: {
    enableShortcuts: false,
    isFullScreen: true,
  },
})

Expected behavior
Shortcuts should be disabled.
Story should be fullscreen.

System:
Version: 5.x

@shilman
Copy link
Member

shilman commented Apr 21, 2019

isFullscreen working for me. What are you seeing?

enableShortcuts isn't working -- seems like a bug.

@dogoku
Copy link
Author

dogoku commented Apr 21, 2019

@shilman I see, you are using isFullscreen not isFullScreen...

In the docs, the parameter is written as isFullScreen but the implementation expects isFullscreen.

I'm not sure which is the "correct", but I suppose updating the docs would be easier.

Edit: isFullScreen is also misspelled here (line 80)

@shilman shilman changed the title enableShortcuts and isFullScreen options not working in v5 enableShortcuts not working in v5 Apr 25, 2019
@shilman shilman added this to the 5.0.x milestone Apr 25, 2019
@stale
Copy link

stale bot commented May 16, 2019

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. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@kaiyoma
Copy link

kaiyoma commented Jun 10, 2019

Adding a comment to beat back the inactivity bot. I find the keyboard shortcuts pretty disruptive, so having a working config option to disable them would be great.

@danyim
Copy link

danyim commented Jun 21, 2019

+1 from me as well. Also, how's it going @shilman 😄 -- saying hi from a distant past from Zippy.

@kane41
Copy link

kane41 commented Jun 22, 2019

+1

@stale stale bot added the inactive label Jul 13, 2019
@storybookjs storybookjs deleted a comment from stale bot Jul 13, 2019
@stale stale bot removed the inactive label Jul 13, 2019
@stale stale bot added the inactive label Aug 3, 2019
@shilman shilman added the todo label Aug 3, 2019
@stale stale bot removed the inactive label Aug 3, 2019
@aaronmw
Copy link

aaronmw commented Aug 27, 2019

Bumping. Storybook's keyboard shortcuts are gobbling all of my components' keypresses. This decoy config option is a day ruiner, big time :(

@radufilipescu
Copy link

Same issue with V6

@Sterv
Copy link

Sterv commented Dec 23, 2020

{ enableShortcuts: false } works for 6.1.10, but does not work for 6.1.11

@yog3sha
Copy link

yog3sha commented Dec 24, 2020

{ enableShortcuts: false } works for 6.1.10, but does not work for 6.1.11

Yes it works in firefox for me, but not in chrome! :-/
Did you try it in chrome?
I tried both

addons.setConfig({
enableShortcuts: false
});

and

addParameters({
enableShortcuts: false
})

separately and together as well, but chrome just won't accept these even after clearing localStorage and memory!!

@antoniosZ
Copy link

Storybook 6.1.15 on Chrome Version 88 and

// preview.js
addParameters({
  ...
  options: {
    enableShortcuts: false,
    ...
  }
});

worked for me

@antoniosZ
Copy link

I updated to latest storybook 6.1.17 and got a warning about this
https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#deprecated-immutable-options-parameters

so now I tried again

// in .storybook/manager.js
import addons from '@storybook/addons';

addons.setConfig({
  enableShortcuts: false,
});

and that worked. no warnings or issues. hopefully, that was it

@shilman
Copy link
Member

shilman commented Feb 15, 2021

Seems like this is fixed in 6.1, closing!

@shilman shilman closed this as completed Feb 15, 2021
@codemile
Copy link

codemile commented May 1, 2021

I'm using 6.2.9 and it doesn't appear to be working for me. The shortcuts are disabled in storybook, but it's still breaking keyboard events on React components.

The following works outside of Storybook, but not inside a story even with shortcuts disabled.

    const onKeydown = useCallback(
        (event: KeyboardEvent) => {
            console.warn({event});
        },
        []
    );

    useEffect(() => {
        console.warn('useEffect');
        document.addEventListener('keydown', onKeydown);
        return () => document.removeEventListener('keydown', onKeydown);
    }, [onKeydown]);

"useEffect" is logged to the console, but onKeydown is never called.
Works fine outside Storybook.

Turning off shortcuts should allow events on the document? or am I misunderstanding what this feature flag does?

@kevinhaas
Copy link

kevinhaas commented May 5, 2021

@codemile I am also seeing this behavior on 6.1.19 now when previously it seemed to disable the Storybook shortcuts but allowed my own components key events to work. Key events are mostly working now, but I'm seeing lots of strange behavior as well.

@Domiii
Copy link

Domiii commented May 30, 2021

@codemile Same here. This is definitely STILL an issue.

I have enableShortcuts disabled. Default SB shortcuts indeed do not work. But my keydown event handlers are ignored.

This issue should be re-opened.

@shilman
Copy link
Member

shilman commented May 30, 2021

@Domiii Can you please create a reproduction by running npx sb@next repro, following the instructions, and linking it in your issue description? We prioritize issues with reproductions over those without. Thank you! 🙏

@hawkticehurst
Copy link

Not sure if this might help for anyone new coming to this issue, but I was running into the same problem in v6.2.9.

The Storybook documentation demonstrates setting the enableShortcuts inside of .storybook/manager.js, which had no effect for me. However, setting the same config inside of .storybook/preview.js as shown below did the trick.

export const parameters = {
	options: {
		enableShortcuts: false,
		... other config ...
	},
	... other config ...
}

@shilman
Copy link
Member

shilman commented Jun 10, 2021

@jonniebigodes looks like a documentation issue ☝️

@jonniebigodes
Copy link
Contributor

@Domiii and @hawkticehurst I wasn't able to reproduce the issue here. If someone could provide a reproduction I'm more than willing to take a look at it and update the documentation as needed.

Here's what I did:

  • Spun up a new React project and installed the latest Storybook version (version 6.2.9).
  • Created the file .storybook/manager.js with the following inside:
import { addons } from '@storybook/addons';

addons.setConfig({
  isFullscreen: false,
  showNav: true,
  showPanel: true,
  panelPosition: 'bottom',
  enableShortcuts: false, // 👈🏼 Storybook shortcuts disabled here
  isToolshown: true,
  theme: undefined,
  selectedPanel: undefined,
  initialActive: 'sidebar',
  sidebar: {
    showRoots: false,
    collapsedRoots: ['other'],
  },
});
  • Updated one of the sample components (src/stories/Page.js) to the following:
import React, { useCallback, useEffect } from "react";
import PropTypes from "prop-types";

import { Header } from "./Header";
import "./page.css";

export const Page = ({ user, onLogin, onLogout, onCreateAccount }) => {

  // event handling
  const onKeydown = useCallback((event) => {
    console.warn(`onKeydown:${event}`);
  }, []);

  // 

  // effect here
  useEffect(() => {
    console.warn("useEffect");
    document.addEventListener("keydown", onKeydown);
    return () => document.removeEventListener("keydown", onKeydown);
  }, [onKeydown]);
  return (
    <article>
      <Header
        user={user}
        onLogin={onLogin}
        onLogout={onLogout}
        onCreateAccount={onCreateAccount}
      />
      {/* 👇 Super simple input to test event handler and if the effect works */}
      <input onKeyDown={onKeydown}></input>
      <section>
        <h2>Pages in Storybook</h2>
        <p>
          We recommend building UIs with a{" "}
          <a
            href="https://componentdriven.org"
            target="_blank"
            rel="noopener noreferrer"
          >
            <strong>component-driven</strong>
          </a>{" "}
          process starting with atomic components and ending with pages.
        </p>
        <p>
          Render pages with mock data. This makes it easy to build and review
          page states without needing to navigate to them in your app. Here are
          some handy patterns for managing page data in Storybook:
        </p>
        <ul>
          <li>
            Use a higher-level connected component. Storybook helps you compose
            such data from the "args" of child component stories
          </li>
          <li>
            Assemble data in the page component from your services. You can mock
            these services out using Storybook.
          </li>
        </ul>
        <p>
          Get a guided tutorial on component-driven development at{" "}
          <a
            href="https://www.learnstorybook.com"
            target="_blank"
            rel="noopener noreferrer"
          >
            Learn Storybook
          </a>
          . Read more in the{" "}
          <a
            href="https://storybook.js.org/docs"
            target="_blank"
            rel="noopener noreferrer"
          >
            docs
          </a>
          .
        </p>
        <div className="tip-wrapper">
          <span className="tip">Tip</span> Adjust the width of the canvas with
          the{" "}
          <svg
            width="10"
            height="10"
            viewBox="0 0 12 12"
            xmlns="http://www.w3.org/2000/svg"
          >
            <g fill="none" fillRule="evenodd">
              <path
                d="M1.5 5.2h4.8c.3 0 .5.2.5.4v5.1c-.1.2-.3.3-.4.3H1.4a.5.5 0 01-.5-.4V5.7c0-.3.2-.5.5-.5zm0-2.1h6.9c.3 0 .5.2.5.4v7a.5.5 0 01-1 0V4H1.5a.5.5 0 010-1zm0-2.1h9c.3 0 .5.2.5.4v9.1a.5.5 0 01-1 0V2H1.5a.5.5 0 010-1zm4.3 5.2H2V10h3.8V6.2z"
                id="a"
                fill="#999"
              />
            </g>
          </svg>
          Viewports addon in the toolbar
        </div>
      </section>
    </article>
  );
};
  • Started Storybook in development mode with yarn storybook, opened the Page story, and press a couple of keys and I get the following:
    storybook-story-disabled-shortcuts

dev-tools

As you can see the event is being captured and logged. And the shortcuts do not work in Storybook.

storybook-shortcuts-disabled-opt.mp4

Took it a bit further and tested it in production mode, by adding http-server and setting it up in my package.json. Ran build-storybook and issued yarn serve-prod (that's the script I have set up). Opened a browser window to http://localhost:8080 (default port assigned by http-server, opened the Page story and I see the exact same thing as above.

Let me know and we'll go from there.

Stay safe

@hawkticehurst
Copy link

@jonniebigodes Wow, thanks for the detailed response!

The project I'm working on is actually built with web components and @storybook/html (apologies for not clarifying that earlier 🤦), so maybe that's where the issue is stemming from? I'd also be very curious to hear what @Domiii's tech stack was.

The project I'm working on is unfortunately still in private development for the time being (it'll be publicly open-sourced in the next few months). However, I'd still be happy to try and spin up a temp project with the same/similar tech stack to see if I can reproduce the issue.

I'll go do that and report back my results.

@jonniebigodes
Copy link
Contributor

@hawkticehurst no need to thank whatsoever. I should be the one thanking you for the follow-up response and willingness to provide us with a reproduction. Let us know once it's up and we'll look into it.

Stay safe

@hawkticehurst
Copy link

@jonniebigodes Alright here's a minimal reproduction of the issue!

https://github.com/hawkticehurst/storybook-shortcuts-repro

Once again, this is built with web components and the @storybook/html package (v6.2.9). In particular, I'm using Microsoft FAST which is a framework for building web components––similar to something like LitElement.

I've included the enableShortcuts config in both .storybook/manager.js and .storybook/preview.js. What you should (hopefully) find is that toggling the config inside of manager.js has no effect, while toggling the config in preview.js works.

Let me know if you have any other questions as you're testing!

@Domiii
Copy link

Domiii commented Jun 19, 2021

I am also very thankful for the pro-active help and your time investment!

Even more so, because I realized, it was my own mistake. Miswrote 'keydown' as 'keyDown'. Fixed it now, and I can confirm that this is not a storybook bug. NOTE: I am using react.

Again, thanks and sorry!

@jonniebigodes
Copy link
Contributor

@hawkticehurst sorry for the late response. But I was able to look into the issue. Going to detail what I did.

  • Cloned your repository.
  • Installed the dependencies.
  • Commented out the option parameter in .storybook/preview.js.
  • Ran npm start and was able to confirm the issue.
  • Stopped Storybook and commented the addons.setConfig() inside .storybook/manager.js and enabled it in .storybook/preview.js and it worked as you've mentioned.
  • As Storybook 6.3 was just released, I took the opportunity to test it, so I created a branch and ran npx sb upgrade to bump Storybook to the latest version.
  • Defaulted to the intended, which is to include the code inside .storybook/manager.js enabled and made one small adjustment to test it out. Turning it into:
// .storybook/manager.js

import { addons } from "@storybook/addons";

addons.setConfig({
  enableShortcuts: true,
  isFullscreen: true, // For testing purposes to see if it will show the UI in full screen.
  showNav: true,
  showPanel: true,
  panelPosition: "bottom",
  sidebarAnimations: true,
  isToolshown: true,
  selectedPanel: undefined,
  initialActive: "sidebar",
  showRoots: true,
});
  • Ran npm start and it yielded the following:
storybook-shortcuts-working-ms-fast-optimized.mp4

As you can see it works as intended. If you could bump your Storybook version and try it on your end to see if the issue is fixed.

One last thing, which I don't think is related to Storybook itself but probably to the framework you're using. If you press for one of the keys bound to Storybook's shortcuts, for instance "a" you'll see that the Addons panel will be enabled/disabled.
Probably it's related to how the framework works with the events and how it's bubbling up.

Let us know if managed to make it work on your end.

Stay safe

@hawkticehurst
Copy link

@jonniebigodes No worries at all! In the scheme of things, I'd say a week is actually a fairly fast response 😉.

But thank you again for checking out the issue and getting back with a descriptive response! I'd be happy to upgrade to 6.3 and get back to you on what the results are. I'm a bit swamped this week, however, so I will likely get back to you sometime next week.

I was also starting to wonder if this issue was potentially a framework thing and how it handles events, but I can try out what you suggested to see what happens.

P.S. Congrats on the 6.3 release 🎉

@hawkticehurst
Copy link

@jonniebigodes My turn to apologize for the late response. 😅

I finally got around to updating the Storybook packages in both the private project I'm working on and the temp project I created for this issue.

Unfortunately, I'm still running into the same problem as described above in both––I've updated the temp project if you want to check it out for yourself. But at this point, I am thinking this really might be an issue with how my framework handles events.

At least the stop-gap solution of setting enableShortcuts within the .storybook/preview.js file still works just fine, so I will continue to stick with that unless there are any updates to this thread or if I run into any new issues.

I'm not sure where this leaves things on your end, but again it might be worthwhile to mention that enableShortcuts can be also be set within preview.js in the Storybook docs in case anyone runs into the same issue as me.

@sourcec0de
Copy link

This issue seems to have cropped back up in sb 7.
None of the proposals above resolve the issue now I'm afraid.

@ndelangen ndelangen reopened this Feb 24, 2023
@ndelangen
Copy link
Member

I think this is a timing issue with the loading of managerEntries happening too late.
Or the manager runtime getting invoked too soon.

@shilman
Copy link
Member

shilman commented Mar 1, 2023

ZOMG!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.58 containing PR #21291 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb@next upgrade --prerelease

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.