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

ItemEmptyのemptyStateIconを外部から設定できるようにする #157

Merged
merged 8 commits into from
Aug 7, 2020

Conversation

maktak1995
Copy link
Contributor

close #155

Comment on lines 113 to 117
emptyTitle?: string;
emptySubtitle?: string;
emptyImage?: string;
emptyImageWidth?: number;
emptyImageHeight?: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

この辺は単一のオブジェクトで良いかもしれないですね。

// ↓<ItemEmpty />のprops
type ItemEmptyProps = {
  title?: string;
  subtitle?: string;
  emptyImage?: string;
  imageWidth?: number;
  imageHeight?: number;
};

ただ破壊的変更になるので

  1. とりあえず追加分のpropsだけオブジェクトで渡せるようにする
  2. 残りのemptyTitle, emptySubTitleもオブジェクトに含める旨のIssueを書いて、同様の破壊的変更Issueと一緒に片付ける

みたいにするのが良さそうな気がしています!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

承知しました!

Copy link
Contributor

@youchann youchann left a comment

Choose a reason for hiding this comment

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

LGTMです!

Issueは「ガーデニング」ラベルで良い気がしています!!

Comment on lines +105 to +111
// TODO: emptyTitle と emptySubtitle もここに移す
type ItemEmptyProps = {
emptyImage?: string;
imageWidth?: number;
imageHeight?: number;
};

Copy link
Contributor

Choose a reason for hiding this comment

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

今は別ですが、最終的にはItemEmpty.tsxからpropsをimportするイメージです!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

了解です!またsubtitleなどをオブジェクトに追加したときに合わせてimportしますね!

@maktak1995 maktak1995 added the ガーデニング ガーデニングデーに対応する優先度の低いやつ label Aug 7, 2020
@maktak1995 maktak1995 merged commit 0ae1134 into master Aug 7, 2020
@maktak1995 maktak1995 deleted the issue-155-changeable-emptyIcon branch August 7, 2020 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ガーデニング ガーデニングデーに対応する優先度の低いやつ
Projects
None yet
Development

Successfully merging this pull request may close these issues.

empty state iconを自由に差し替えられるようにする
2 participants