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

Essentials: Add measure and outline addons #15107

Merged
merged 5 commits into from
Jun 2, 2021
Merged

Conversation

winkerVSbecks
Copy link
Contributor

Issue:

What I did

Added the Measure and Outline addons to @storybook/addon-essentials

How to test

Build @storybook/addon-essentials and load up any of the example Storybooks

  • Is this testable with Jest or Chromatic screenshots?
    No

  • Does this need a new example in the kitchen sink apps?
    No

  • Does this need an update to the documentation?
    Yes, should add Measure and Outline to the list of essential addons

If your answer is yes to any of these, please make sure to include it in your PR.

@nx-cloud
Copy link

nx-cloud bot commented May 31, 2021

Nx Cloud Report

CI ran the following commands for commit d302502. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

yarn.lock Outdated Show resolved Hide resolved
@yannbf
Copy link
Member

yannbf commented May 31, 2021

This is super awesome! The only concern I have, which I don't really know if it's a valid one, is that now with these addons, out of the box, Storybook already contains 5 toolbar icons. As soon as people start adding a couple more addons, it gets really crowded. Example of a crowded toolbar (3 from essentials, 3 custom):

image

I'm unsure whether having too many items there becomes a challenging UI for the user. I guess it is, when people access via mobile. What are your thoughts?

@winkerVSbecks
Copy link
Contributor Author

winkerVSbecks commented Jun 1, 2021

@yannbf the user always has the option to disable essential addons that they are not using. But, I think in the long term we need a tweak to the toolbar to support more addons.

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.

A couple minor issues

addons/essentials/package.json Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
@winkerVSbecks
Copy link
Contributor Author

@shilman updated, could you take another look please

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.

@winkerVSbecks There are a couple issues here.

  • the PnP tests are failing because addon-outline uses ts-dedent but doesn't declare it as a dependency in package.json. Adding it & republishing the addon should fix it.
  • Chromatic is failing on some measure-related error

Home stretch!

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.

Looking good! I checked the perf and there is a slight uptick in install size & manager bundle size, which is to be expected. Merging & will release later today.

@shilman shilman changed the title Add measue and outline addon to essentials Essentials: Add measue and outline addons Jun 2, 2021
@shilman shilman merged commit d7fd97b into next Jun 2, 2021
@shilman shilman deleted the addon-css-essentials branch June 2, 2021 15:53
@shilman shilman changed the title Essentials: Add measue and outline addons Essentials: Add measure and outline addons Jun 2, 2021
@IanVS
Copy link
Member

IanVS commented Jun 3, 2021

It might be good to update the docs (or at least readme) too, it seems there are a few more essentials than are indicated on https://storybook.js.org/addons/tag/essentials/ and https://github.com/storybookjs/storybook/tree/next/addons/essentials#contents.

@shilman
Copy link
Member

shilman commented Jun 3, 2021

Thanks @IanVS. We'll be sure to update everything as part of the 6.3 release cc @winkerVSbecks @jonniebigodes

@hckhanh
Copy link

hckhanh commented Jun 9, 2021

Why this cool feature is disabled in rc.0 version?

@shilman
Copy link
Member

shilman commented Jun 9, 2021

@hckhanh it's enabled in rc.0. screenshot from a fresh install:

Example___Button_-_Primary_⋅_Storybook

@hckhanh
Copy link

hckhanh commented Jun 10, 2021

Hi @shilman, I don't know why but when I upgrade from beta.12 to rc.0. The Measure and Outline tools are disappeared. I leave my configurations bellow:

image

main.js

module.exports = {
  stories: ["../src/**/*.stories.mdx", "../src/**/*.stories.@(js|jsx|ts|tsx)"],
  addons: [
    "@storybook/addon-links",
    "@storybook/addon-essentials",
    "@storybook/preset-create-react-app",
  ],
  core: {
    // builder: 'webpack5',
  },
};

manager.js

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

addons.setConfig({
  theme: themes.dark,
});

preview.js

import { themes as storybookThemes } from "@storybook/theming";
import { ThemeProvider } from "styled-components";
import themes, { GlobalStyles } from "../src/themes";

export const parameters = {
  actions: { argTypesRegex: "^on[A-Z].*" },
  controls: {
    matchers: {
      color: /(background|color)$/i,
      date: /Date$/,
    },
  },
  docs: {
    theme: storybookThemes.dark,
  },
};

export const decorators = [
  (Story) => (
    <ThemeProvider theme={themes}>
      <GlobalStyles />
      <Story />
    </ThemeProvider>
  ),
];

@shilman
Copy link
Member

shilman commented Jun 10, 2021

@hckhanh Did you try removing node_modules and/or lockfiles and reinstalling?

@hckhanh
Copy link

hckhanh commented Jun 11, 2021

Ops I am using Yarn 2 with plug'nplay

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