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

Refactor Storybook <Button /> #213

Merged
merged 12 commits into from
Dec 7, 2020
Merged

Refactor Storybook <Button /> #213

merged 12 commits into from
Dec 7, 2020

Conversation

youchann
Copy link
Contributor

@youchann youchann commented Nov 30, 2020

ref: #212

ここを親ブランチとし、**.stories.tsxをリプレイスする際は別ブランチからこちらにマージする。
全てのコンポーネントを書き換えた時点で当PRをマージする。

コンポーネントの数が多いので、随時リリースする方針に変更。
なお、進捗は #212 の方で管理する。

やったこと

  • @storybook/addon-console@storybook/addon-consoleの導入
  • Button.stories.mdxにドキュメンテーション
  • .scaffdog/component.md.mdxに対応させる
  • 既存の**.stories.tsxにおいて、Docsタブをみれないように
  • ESLintが.mdx対応できるように

@youchann youchann added the enhancement New feature or request label Nov 30, 2020
@youchann youchann requested a review from a team as a code owner November 30, 2020 09:02
@youchann youchann self-assigned this Nov 30, 2020
@youchann youchann requested review from penicillin0 and removed request for a team November 30, 2020 09:02
Comment on lines +9 to +11
// TODO: fix warn "Rendered more hooks than during the previous render."
// knobを完全に排除できたタイミングで再度調査
// strictMode: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

memo

@youchann youchann removed the request for review from penicillin0 November 30, 2020 09:11
@@ -58,6 +58,7 @@ export default {
title: "NavigationRail",
parameters: {
component: NavigationRail,
layout: "fullscreen",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

デフォルトではcanvasにpaddingが付与されているが、サイドバーなどのコンポーネントにとっては不要。

@youchann youchann changed the title wip Refactor Storybook wip Refactor Storybook <Button /> Dec 7, 2020
Comment on lines +73 to +75
argTypes={
{ onClick: { action: "clicked" } }
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

<Canvas>
<Story
name="something"
args={{}}
Copy link
Contributor Author

@youchann youchann Dec 7, 2020

Choose a reason for hiding this comment

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

実際に生成するとargs= みたいな感じで、何も書かれない。
args={{{{}}}}みたいに書くとシンタックスエラーみたいなのが出る

@@ -1,5 +1,7 @@
import * as React from "react";
import { ThemeProvider, createTheme } from "../src/themes";
import "@storybook/addon-console";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +10 to +12
parameters: {
docs: { page: null },
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

現行の.stories.tsxはDocsタブ常にデフォルトのものが表示されるが、

  • 情報量が足りない
  • コンポーネントによっては見辛い

ということで共通で見れないようにした。

Comment on lines +18 to +36
<Canvas>
<Story
name="primary"
args={{
color: "primary",
children: "primary",
}}
>
{(args) => <Button {...args} />}
</Story>
<Story
name="secondary"
args={{
color: "secondary",
children: "secondary",
}}
>
{(args) => <Button {...args} />}
</Story>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

.map()で書きたいが、今の所かけない。

ref: storybookjs/storybook#8392

Comment on lines -26 to -28
<Fade timeout={{ enter, exit }} in={isOpen}>
<Button>Control in Knob Footer</Button>
</Fade>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Storybookアプデの影響か、このコンポーネントにおいてはKnobの操作によってtransitionが発火しなかった。

ので、<Grow />と同じ構成にした。

@youchann youchann changed the title wip Refactor Storybook <Button /> Refactor Storybook <Button /> Dec 7, 2020
Comment on lines +5 to +7
"eslint.options": {
"extensions": [".js", ".jsx", ".md", ".mdx", ".ts", ".tsx"]
},
Copy link
Contributor Author

@youchann youchann Dec 7, 2020

Choose a reason for hiding this comment

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

いずれ.mdxが対応されるはずなので、あらかじめいれておいた。

@youchann youchann mentioned this pull request Dec 7, 2020
53 tasks
@youchann
Copy link
Contributor Author

youchann commented Dec 7, 2020

@ryokosuge
必要であれば口頭で共有しましょう:+1:

component: {{ input }},
};
<Meta
title="{{ input | pascal }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

めちゃくちゃ細かいんだけど

https://storybook.js.org/docs/react/writing-docs/mdx#writing-stories

<Meta
    title=""
>

のtitleのところを例えば Component/{{ input | pascal }} みたいにディレクトリ切る感じで書いていくと

スクリーンショット 2020-12-07 15 03 03

こんな感じ(ここだと <Meta title="MDX/Badge">)になるんだけど、分けていった方がみやすいかな?って思ったんだけどどうでしょう!

  • Component
  • Layout

とかくらいに分かれるかなーっていうイメージ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ああ、全然良いと思います:+1:

別PRでやりましょうか !!

Copy link
Contributor

Choose a reason for hiding this comment

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

そうしましょう!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

この辺にちょろっと書いた
#212 (comment)

name="something"
args={{}}
>
{(args) => <{{ input | pascal }} {...args} />}
Copy link
Contributor

Choose a reason for hiding this comment

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

<!--- This is your Story template function, shown here in React -->

export const Template = (args) => <Badge {...args } />

Buttonのやつみる限り、複数回 {(args) => <{{ input | pascal }} {...args} /> これを書いてるから、{Template.bind({})} の方がちょっと楽かな?と思い!
使わなかった理由があったら教えて欲しい!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

良い質問ですね!!(書き忘れていた)

時間あれば手元でやっていただけると嬉しいのですが、出力されるコピペ可能なコードにおいて、Buttonというコンポーネント名で出力されなかったんですよね.....
なのでベタがきしている。といったかんじです!

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

自分の手元で再現してみた。

image

Copy link
Contributor

Choose a reason for hiding this comment

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

なるほど、MDXCreateElementは export const Template = (args) => <Badge {...args } /> で出てくるのか!
だったら PreviewのところはちゃんとしたComponent名で出て欲しいから、この記法の方が良さそうだね!

"@storybook/addon-storysource": "^6.1.6",
"@storybook/addon-console": "^1.2.2",
"@storybook/addon-essentials": "^6.1.9",
"@storybook/addon-knobs": "^6.1.9",
Copy link
Contributor

Choose a reason for hiding this comment

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

こいつ残してるのは、前のやつとの互換とるためかな?
knobsとessentialsに含まれてるcontrolsがほぼ同じやつだと認識してるんだけど、もしかして理解足りてない...?w

Copy link
Contributor Author

@youchann youchann Dec 7, 2020

Choose a reason for hiding this comment

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

そうですね。前のやつと互換を取るためです!

knobsとessentialsに含まれてるcontrolsがほぼ同じやつだと認識

合ってます!がライブラリとしては別モノなので、一気に乗り換えるためには修正は必要でして。。。
https://github.com/storybookjs/storybook/tree/next/addons/controls#how-do-i-migrate-from-addon-knobs

いずれ全て変わるので、一気にやらなくても良いかなという判断です!

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@ryokosuge ryokosuge self-requested a review December 7, 2020 06:37
Copy link
Contributor

@ryokosuge ryokosuge left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@youchann youchann merged commit edd4387 into master Dec 7, 2020
@youchann youchann deleted the refactor-storybook branch December 7, 2020 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants