-
Notifications
You must be signed in to change notification settings - Fork 0
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
File select #37
File select #37
Conversation
app/src/core/components/FileList.tsx
Outdated
|
||
const FileListImage = styled.img` | ||
width: 40px; | ||
height: 40px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: ideally we'd use theme
sizing values for stuff like this, for easier customization across projects, but if we cant, imo arbitrary exact values should use rem
units except for precision pixel placements, so we get screen zooming/scaling support for free.
app/src/core/components/FileList.tsx
Outdated
|
||
interface FileListProps { | ||
files: FileWithProgress[]; | ||
onRemove: (file: File) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nit items,
I'd think we'd want an onComplete
callback, or something to talk back to a given parent. Some sort of established outward communication. Also perhaps a file count or something? Perhaps some kind a pub/sub progress even...though that seems a bit too specifi as that only makes a ton of sense for something of any meaningful size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about some kind of flow where something happens on upload, or something happens after someone uploads a set of files (like...I dunno. "Please upload your license front and back" and then we'd know if 2 images were uploaded)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileList is just a list and doesn't have any completion. I was specific about trying to not include any actual file upload stuff in this one, as its just for selecting files. Providing the progress is a way to just update the progress indicator for the specific file and whatever is passing it in will know when its complete or not
app/src/core/types/theme.ts
Outdated
// (height: SizingArgument): string; | ||
// (width: SizingArgument, height: SizingArgument): string; | ||
// } | ||
// export type SizingArgument = number | string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want this commented out code left in?
This PR adds the ability to select files and defaults the functionality across our apps.