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

FileUploader Componentを追加 #203

Merged
merged 11 commits into from
Nov 27, 2020
Merged

FileUploader Componentを追加 #203

merged 11 commits into from
Nov 27, 2020

Conversation

maktak1995
Copy link
Contributor

@maktak1995 maktak1995 self-assigned this Nov 24, 2020
<Flex display="flex">
<Icon name="folder_open" size="md" color="active" />
<Typography weight="bold" color="primary">
ドラッグ&ドロップするか、クリックしてアップロード
Copy link
Contributor

Choose a reason for hiding this comment

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

超長期でみれば、この文言はpropsで管理してあわよくば初期値は英語にしてしまいたい思いがありますw

onSelectFile: (files: File) => void;
};

const FileUploader: React.FunctionComponent<FileUploaderProps> = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

やろうとしていること自体は単純で、ファイルを引数にコールバック叩ければ良いので、ライブラリを使うまでもないのでは...:thinking:と思っていたのですが、とっても良いサンプル見つけました。

https://github.com/kufu/smarthr-ui/tree/master/src/components/DropZone

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.

確かにそうですね!
自前でかいちゃいます!

@maktak1995 maktak1995 requested a review from a team as a code owner November 27, 2020 05:00
export type FileUploaderProps = {
description?: string;
title?: string;
accept?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

なんか型で制御出来たら嬉しいなって思ったけど、そこら辺おいしいやつなさそうよね。。。

https://developer.mozilla.org/ja/docs/Web/HTML/Element/input/file#Unique_file_type_specifiers

Copy link
Contributor

Choose a reason for hiding this comment

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

うん、ごめん、TypeScriptの HTMLInputElementacceptstring だったから string でおいといて良さそうですね!

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 👍

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です!

Comment on lines +11 to +12
width: 564px;
height: 144px;
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.

変更したい場合はStyle当ててもらえばいいかなーと思ったのですが、やはり引数で指定できるようにしたほうがいいですかね...?

Copy link
Contributor

Choose a reason for hiding this comment

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

使う側は引数で利用した方がラクなのでその方が良いです!

が、今回は要素の中身(文言の大きさや周辺の余白)が固定なので幅とかいじっても見辛くなりそうですよね。
この辺りは @noronaoki さんとディスカッションした方が良いかもです(大きさを変えたいユースケースがあったりするのかとか)

@maktak1995 maktak1995 merged commit e7e963b into master Nov 27, 2020
@maktak1995 maktak1995 deleted the addFileUploaderComponent branch November 27, 2020 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants