Skip to content

feat(SimpleFileUpload): consumed Penta tokens#6211

Merged
mcoker merged 3 commits into
patternfly:v6from
thatblindgeye:fileUploadPenta
Jan 12, 2024
Merged

feat(SimpleFileUpload): consumed Penta tokens#6211
mcoker merged 3 commits into
patternfly:v6from
thatblindgeye:fileUploadPenta

Conversation

@thatblindgeye

Copy link
Copy Markdown
Contributor

Closes #6209

  • Removed usage of the form helper text since it seems like from the design the helper text should be tied to File Upload, not Form
  • Added a helper text example that makes the text area aria label "Uploaded file content" and tied the helper text to the Upload button. Also made these updates to the "with error" example. As a follow up we should consider making similar changes to other examples, as well as:
    • possibly updating the text input aria label to "File name"; right now it just duplicates the placeholder or value attribute
    • remove the aria-describedby from the text input, as currently it being described by the upload button doesn't provide adequate description

For the "with error" example, is the intent that if a file upload has a type/size restriction, it must be placed in a Form component? Or is this example more like a demo of showing how one could place a file upload inside a form? Right now React only uses Form components for a similar "restrictive" example, but it would seem like a non-restrictive file upload could/should also be placed inside a Form for some sort of submission as well.

@thatblindgeye thatblindgeye requested review from a team, andrew-ronaldson, lboehling, mcoker and srambach and removed request for a team January 11, 2024 16:40
@patternfly-build

patternfly-build commented Jan 11, 2024

Copy link
Copy Markdown
Collaborator

@lboehling lboehling left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM!!

@srambach srambach 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.

🌟 Looks great to me

@andrew-ronaldson andrew-ronaldson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Happy happy joy joy!

@mcoker mcoker left a comment

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.

Just one nit!

Comment thread src/patternfly/components/FileUpload/file-upload.scss Outdated

@mcoker mcoker left a comment

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.

🔥

@mcoker mcoker merged commit f3a4a6b into patternfly:v6 Jan 12, 2024
@patternfly-build

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 6.0.0-alpha.58 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants