Skip to content

feat(spindle-ui): convert SVG icon to React component#7

Merged
herablog merged 7 commits into
masterfrom
feature/ui-icon
Aug 27, 2020
Merged

feat(spindle-ui): convert SVG icon to React component#7
herablog merged 7 commits into
masterfrom
feature/ui-icon

Conversation

@herablog

@herablog herablog commented Aug 5, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

SVGRを使って、SVGからReactコンポーネントに変換しました。Spindleコンポーネント内でアイコン利用することがあるので、コンポーネント化しています。

SVGR Options

  • typescript : 他のコンポーネントに合わせてTypeScriptとして書き出します
  • icon: 親要素のフォントサイズを継承するため、1emをデフォルトサイズとして指定します
  • ignoreExisting: アイコン生成時に変更内容が反映されるようにします。差分はgitで管理します。

Storybook

icon-storybook

@herablog herablog requested a review from tokimari August 5, 2020 08:57
@herablog herablog self-assigned this Aug 5, 2020
@herablog herablog force-pushed the feature/ui-icon branch 2 times, most recently from 3ed3eb0 to 6fc23e0 Compare August 5, 2020 09:02
herablog
herablog commented Aug 5, 2020
View reviewed changes
Comment thread packages/spindle-ui/package.json
"build:style": "postcss src/**/*.css src/*.css --base src -d dist/"
"build:style": "postcss src/**/*.css src/*.css --base src -d dist/",
"preicon": "npx cpx '../spindle-icons/dist/!(sprite).svg' icon-tmp",
"icon": "svgr --out-dir src/Icon icon-tmp",

@herablog herablog Aug 5, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

spindle-iconの更新と同じタイミングで実行するコマンドです。どこかしらで自動にするかは検討中・・・。

@herablog herablog mentioned this pull request Aug 12, 2020
Merged
1 task
tokimari
tokimari reviewed Aug 17, 2020
View reviewed changes
Comment thread packages/spindle-ui/src/Icon/index.stories.tsx
<div>
<Clock height="200" width="200"></Clock>
</div>
);

@tokimari tokimari Aug 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

代替テキストありverのstoryもあると嬉しいです！

@herablog herablog Aug 17, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

利用サンプルとしての属性をたしました〜 b5688ad

ボタン内で利用する際のサンプルは、<Button>の方に書く予定です。

@tokimari tokimari Aug 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

あ、むしろ role="img" は常に（デフォルトで）つけた方がいいかもです><ｲﾏｻﾗ
代替テキストは不要なケースもありそうだけれども・・！

@herablog herablog Aug 17, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

そりね〜、デフォルトで入れるか、使う時に決めてもらうかと思ってたけど、Icon前提だから入れておきますか〜。

@herablog herablog Aug 18, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

デフォルトのroleをimgにしました。
3fa1e2d

いちお、利用時に上書きも可能です。

@herablog herablog requested a review from sasaplus1 August 26, 2020 08:24
@sasaplus1

sasaplus1 commented Aug 26, 2020

Copy link
Copy Markdown
Member

Storybook開こうとしたらおこられました 😢

おこられ

@herablog

herablog commented Aug 26, 2020

Copy link
Copy Markdown
Member Author

おこられ発生!! こりはもしや、生成からやってくれましたか!? なぜか ArrowDown が 複数書き込まれちゃう問題が・・。

@herablog

herablog commented Aug 26, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

わかった!! こっちが原因だ・・ｗ arrow-down.svg, arrow_down.svg

https://github.com/openameba/spindle/tree/master/packages/spindle-icons/dist

@sasaplus1

sasaplus1 commented Aug 26, 2020

Copy link
Copy Markdown
Member

yarn storybook:startしておこられ発生して、あれ〜と思ったのでyarn buildしてみてもおこられ発生って感じでしたw

とりあえずArrowDownを手動で1行消してStorybook見れました〜

@herablog

herablog commented Aug 26, 2020

Copy link
Copy Markdown
Member Author

おかげさまで問題を発見できましたｗ SVGの方修正しますｗ

@herablog herablog mentioned this pull request Aug 26, 2020
Merged
@herablog

herablog commented Aug 26, 2020

Copy link
Copy Markdown
Member Author

@sasaplus1 #15 でSVGの修正をしました。ただこの間にアイコン更新されてたので、それも含めてになっちゃいました。(便乗PRですみませんｗ)

@herablog

herablog commented Aug 26, 2020

Copy link
Copy Markdown
Member Author

#15 を取り込みつつ、以下の変更を加えました。

  • Icon書き出し直し a72dd99
  • 必要ないファイルが残らないように、Iconファイルを作り直す前に消す c1d2239
  • Iconのentry fileであるindex.js のバジェット更新 1910abd
    • このファイルはあまり意味ないと言うか、アイコン数と連動しますが・・

sasaplus1
sasaplus1 approved these changes Aug 27, 2020
View reviewed changes

@sasaplus1 sasaplus1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

とりあえず無事にStorybook見れました〜:tada:

@herablog

herablog commented Aug 27, 2020

Copy link
Copy Markdown
Member Author

おーありがとうございます！

@herablog herablog merged commit b1b5e3f into master Aug 27, 2020
@herablog herablog deleted the feature/ui-icon branch August 27, 2020 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tokimari tokimari tokimari left review comments
@sasaplus1 sasaplus1 sasaplus1 approved these changes

Assignees

@herablog herablog

Labels

spindle-ui

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@herablog @sasaplus1 @tokimari