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

Addon API: Add experimental page addon type #23307

Merged
merged 37 commits into from Jul 13, 2023

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Jul 4, 2023

What I did

  • Introduce a new addon type: page (prefix with experimental for now)
  • Simplify the API for this new addon type
  • Make API for addon injection stricter
  • Fix a bug where selection during preview initialization was ignored
  • Make the default route / even if there's no query
  • Refactor the layout to hide the storybook main area when the route doesn't match (as apposed to the exception /settings was before)

How to test

  • create a sandbox
  • add a custom manager entry
  • here's the code I've been using to test with:
    import type { ComponentProps, FC } from 'react';
    import React, { memo, Fragment } from 'react';
    import { addons, types } from '@storybook/manager-api';
    
    import { Icons, IconButton, Bar, TabButton } from '@storybook/components';
    import { styled } from '@storybook/theming';
    import { Route } from '@storybook/router';
    
    const Toolbar = styled(
      ({ shown = true, ...props }: ComponentProps<typeof Bar> & { shown: boolean }) => (
        <Bar {...props} />
      )
    )(
      {
        position: 'absolute',
        left: 0,
        right: 0,
        top: 0,
        transition: 'transform .2s linear',
      },
      ({ shown }) => ({
        transform: shown ? 'translateY(0px)' : 'translateY(-40px)',
      })
    );
    
    const FrameWrap = styled.div<{ offset: number }>(({ offset }) => ({
      position: 'absolute',
      overflow: 'auto',
      left: 0,
      right: 0,
      bottom: 0,
      top: offset,
      zIndex: 3,
      transition: 'all 0.1s linear',
      height: `calc(100% - ${offset}px)`,
      background: 'transparent',
    }));
    
    const Centered = styled.div({
      position: 'relative',
    
      width: '100%',
      height: '100%',
      display: 'flex',
      alignItems: 'center',
      justifyContent: 'center',
    });
    
    addons.register('@storybook/addon-debugger', (api) => {
      addons.add('@storybook/addon-debugger/tool', {
        title: 'debugger!',
        type: types.TOOLEXTRA,
        match: ({ viewMode }) => !!(viewMode && viewMode.match(/^(story|docs)$/)),
        render: () => (
          <Fragment>
            <IconButton
              key="debugger"
              title="navigate to debugger-view"
              onClick={() =>
                api.navigateUrl(`/debugger/${api.getCurrentStoryData().id}`, { plain: false })
              }
            >
              <Icons icon="lightning" />
            </IconButton>
          </Fragment>
        ),
      });
    
      const DebuggerPage: FC = () => {
        return (
          <Route path="/debugger" startsWith>
            <Toolbar shown border>
              <TabButton active>A tab</TabButton>
              <IconButton
                key="first"
                title="Go to first story, for some reason. It's just a demo."
                onClick={() => api.selectFirstStory()}
              >
                <Icons icon="star" />
              </IconButton>
            </Toolbar>
            <FrameWrap offset={40}>
              <Centered>
                This is the contents of my addon, in a full viewport experience, what a joy!
              </Centered>
            </FrameWrap>
          </Route>
        );
      };
    
      addons.add('@storybook/addon-debugger/panel', {
        title: 'Debugger',
        type: types.experimental_PAGE,
        url: '/debugger',
        render: memo(DebuggerPage),
      });
    });
  • navigate to /: expect the story to be shown
  • navigate to /settings/about : expect the about page to be shown
  • navigate to /debugger : expect the debugger panel to be shown
  • click on the ⭐ icon in the toolbar (whilst on the debugger page : expect the route to change, and the first story to be shown
  • use the browser back button : expect the debugger page to be shown again
  • use the browser forward button : expect the story to be shown again
  • click the ⚡ icon on the right side of the toolbar (whilst on the canvas) : expect the route to change and the debugger page to be shown

I have done the best I can to have some of this also make sense for the mobile view, but this is really up for grabs. We need to determine what to do regarding pages for mobile.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

Base automatically changed from norbert/ui-tabs-types-improvements to release/7.2 July 6, 2023 22:29
code/lib/manager-api/src/index.tsx Show resolved Hide resolved
code/lib/manager-api/src/modules/addons.ts Outdated Show resolved Hide resolved
code/lib/manager-api/src/modules/stories.ts Outdated Show resolved Hide resolved
code/lib/manager-api/src/modules/stories.ts Outdated Show resolved Hide resolved
code/lib/types/src/modules/addons.ts Show resolved Hide resolved
@tmeasday
Copy link
Member

It worked well but I did see these warnings:

image

@ndelangen ndelangen requested a review from tmeasday July 11, 2023 13:19
@shilman shilman changed the title AddonApi: Add a new addon type: page (experimental) Addon API: Add experimental page addon type Jul 11, 2023
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

During the review you mentioned that there is a breaking TS change where Addon_Type's type field is now required. Please add this to MIGRATION.md.

I love the use of experimental_ in the name. Playwright also does this and I think it's a great convention -- we should do more.

@ndelangen
Copy link
Member Author

@shilman I don't think the Addon_BaseType needs to be documented in the migration guide.. it is only relevant if you're consuming registered addon, not when injecting them.

I did end up writing some migration guide bits for the increased strictness of the manager-api

@ndelangen ndelangen requested a review from tmeasday July 12, 2023 13:07
@ndelangen ndelangen merged commit 82afbe6 into release/7.2 Jul 13, 2023
51 checks passed
@ndelangen ndelangen deleted the norbert/page-addons-refactor branch July 13, 2023 07:54
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.

None yet

3 participants