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 upload #161
Comments
Background:
|
@ajacobs21e these look great. We don't have to include this for the PF4 version, because I know it's not part of your use case, but I wonder if we could have a variant that displays the contents of the file after uploading, which could be the same layout we offer the user to allow them to paste in a file. Also, it looks like your use case is for PF4 and mine is for PF3, so maybe we won't end up sharing code here, but I'd be happy to contribute to both implementations. |
Ideally we can come up with a design that works for all use cases and for both PF3 & PF4 |
Here's what @mturley did for PF3, though it was never accepted into PF. patternfly/patternfly-react#1254 |
Marvel prototypes: https://marvelapp.com/de0ggah |
Updated mocks based on feedback from today: https://marvelapp.com/de0ggah |
@lboehling This looks great, and I think we are just about ready to hand off to development. The only question I have if we need to specify how progress is reflected as the file uploads. I suspect that in most cases it will be quick, but possibly there could be some lag if we were loading over a network, for example. Thoughts on the approach? Integrate a spinner possibly? Perhaps just add this to the Marvel deck. @mturley can you review the design and let us know if you see any issues? I'm also unclear if we need to start this in Core or can go directly to creating a React component. @mcoker do you see anything here that would require unique HTML/CSS or can we build from existing components? |
@mcarrano doesn't look like this could be built without custom styles on top of existing components, so we would want to build it in core to best support the layout and interaction styling. |
@mcarrano the designs look great to me! The only thing I noticed is that the filename bar is editable, I think we at least need to make it possible to disable / grey out (probably comes for free with existing core styles for a text field though). If the user is browsing/drag-and-dropping a file, it'll prefill that filename just to show what was selected/dropped, it's confusing if you can change it since that has no effect (at least in the use case I had where only the contents end up being submitted). The other restriction there is that if the user wanted to type in a file path instead of using the browse button (not sure why they would...) I don't think that would work with JS's FileReader, although I could be wrong. As long as that field can be disabled, we can figure that out at JS development time. I agree with @mcoker that this should start in core first. |
I also want to put in my vote for this to be called "Text File Upload" specifically, unless we plan to have a variant where the preview box can be hidden, or show an image, or show metadata about a binary file of some other format. |
OK. Looks like there are just a couple of small details to clear up. I'm also seeing that the text area is presented as disabled and probably shouldn't be when text is pasted in as I'd assume that we'd want to allow users to edit that. I have created a new core issue: patternfly/patternfly#2624 to get the core CSS work started and will schedule time to identify a final set of changes before we hand this off. |
Marvel link is updated: https://marvelapp.com/de0ggah |
Looks great @lboehling . Thanks for your efforts on this! |
Create a template with this flow: https://marvelapp.com/de0ggah/screen/65737568. Sketch file: https://sketch.cloud/s/kROPo (Don't think additional symbols are needed here but if they are, comment on this!) |
@ajacobs21e Now that I'm looking at this - I'm wondering if we should use the existing input field symbols we have to recreate the browse and clear fields (even though these are technically buttons). Also in dropdowns we have split buttons that we could possibly leverage instead? Symbol library wise we'll definitely need 1 new style for the blue square that is overlaid on the expandable text box once an item is hovered over it! |
@ajacobs21e the core implementation is already up: https://www.patternfly.org/v4/documentation/core/components/fileupload I think the line you are referring to is implicitly provided by the border-bottom on the read-only text field (read-only as opposed to disabled, which doesn't have that border) and the buttons with the class |
@gdoyle1 I added symbols for all of the states for an expandable text field (although should we rename this to text area?) |
@ajacobs21e That's a good question - I think I like text area since it captures both inputting text and dropping something within it. Do you want to make that change or should I? And I think what you have in the template example is perfect! |
and it matches https://www.patternfly.org/v4/documentation/react/components/textarea ! |
updated library and template files |
Here are some options I played with for a Portal project. I didn't include pasting since that's not a use case for me.
The left side shows 3 ideas. I like the concept of making it one regular height input field but it might be too cramped. I imagine with pasting capability it would grow in height after pasting.
The middle one might work best - there's an explicit button for browse, a larger drag and drop target area, and then clicking anywhere that's not the button could be used for pasting. It's a little big though.
The third one copies the PF3 version most but I prefer having a button instead of a link for browse. A button is more appropriate for an action and it's more obvious what to do in this field.
On the right shows the flow of uploading and then the success message of uploading a file. That takes care of the questions about how to see the file name and how to remove it.
@mturley and I plan on talking through this more and trying to come up with a single solution for all of these use cases.
Originally posted by @ajacobs21e in patternfly/patternfly-react#1254 (comment)
The text was updated successfully, but these errors were encountered: