8000 feat(SimpleFileUpload): consumed Penta tokens by thatblindgeye · Pull Request #6211 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

feat(SimpleFileUpload): consumed Penta tokens#6211

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

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

Conversation

@thatblindgeye
Copy link
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 8000
Copy link
Collaborator
patternfly-build commented Jan 11, 2024

Copy link
@lboehling lboehling 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
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

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

🌟 Looks great to me

Copy link
Collaborator
@andrew-ronaldson andrew-ronaldson left a comment

Choose a reason for hiding this comment

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

Happy happy joy joy!

Copy link
Contributor
@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

Just one nit!

Copy link
Contributor
@mcoker mcoker left a comment

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
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

0