Add Avatar and ImageInput components #922
Conversation
🦋 Changeset detectedLatest commit: eb3121e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/sumup/oss-circuit-ui/5ZiBd1WtwfG7FmWJS2sxwhKqGswd |
|
Hey @robinmetral, Thanks! |
Codecov Report
@@ Coverage Diff @@
## main #922 +/- ##
==========================================
+ Coverage 91.85% 91.99% +0.13%
==========================================
Files 161 163 +2
Lines 2923 3009 +86
Branches 759 784 +25
==========================================
+ Hits 2685 2768 +83
- Misses 209 212 +3
Partials 29 29
|
A file input is always uncontrolled so a ref is necessary here: https://reactjs.org/docs/uncontrolled-components.html\#the-file-input-tag
- the border-radius of the zetta size was increased to 12px, the value will be moves to design tokens and used in the next Circuit major version - a bug on Chrome where uploading the same image twice would not fire onChange the second time was fixed by clearing the DOM input element on click - the AvatarInput was adapted to accept any component as a visual element, the Avatar or something else. A new story "Custom Component" was added to document it. I will add tests for it and rename AvatarInput back to ImageInput in follow-up commits
Since the last commit, it accepts any visual element, although it is still optimized for the Avatar component. This will enable teams to use the ImageInput for use cases beyond an avatar upload.
robinmetral
changed the title
This is to ensure that the button still stands out well even with invalid styles (from a design feedback)
- This makes the component's implementation cleaner - It makes the tests cleaner because there's only one matching label that we can query with getByLabelText - This commit also adds missing prop descriptions
| - **Do** use the right variant for your use case (see _Variants_ below). | ||
| - **Do** use the [ImageInput component](Forms/ImageInput) with the `component={Avatar}` prop to allow users to upload an avatar (note: the ImageInput only supports `variant="object"` for now). | ||
|
|
||
| ## Accessibility |
There was a problem hiding this comment.
Thanks for adding a dedicated Accessibility section! 🙌🏻
Some feedback: I would flip the order of the recommendations: adding alt text should be the default, omitting it the exception. It's better to have duplicate content than missing content.
There was a problem hiding this comment.
I reworded it to put more focus on when the alt text was necessary.
Do you think we should make alt a required prop and remove the "" default? In that case developers will need to explicitly pass "" if their images are presentational.
There was a problem hiding this comment.
I think that's a good idea. It forces developers to make a conscious decision.
|
@connor-baer I addressed your comments (thank you! 🙌), let me know what you think about the last two open questions (I believe):
|
The ImageInput also exposes it as part of the custom component's signature, but it defaults to in that case. Accessibility docs were also updated and an extra usage example with a custom component was added to the ImageInput stories.
Addresses #722 (partly).
Purpose
Introduces the Avatar and ImageInput components.
The Avatar component is a visual element with two variant and two sizes, used to display either identity (e.g. profile image) or object (e.g. product image) avatars.
The ImageInput receives a visual element (with drop-in support for the Avatar) and adds file input logic as well as state-specific styles.
Approach and changes
High-level, the Avatar is a purely presentational element, while the ImageInput handles state-specific styles (hover, active, invalid) as well as file input logic. The actual uploading of an image should be handled in each application via an onChange callback passed to the ImageInput.
Refer to the Storybook for further implementation and usage guidelines, and to the threads below for more details and discussion on the implementation.
Note: some things have been left out of this PR's scope and will be addressed in later versions (primarily for versioning reasons):
objectvariant@sumup/icons(major)@sumup/design-tokens(major)Definition of done