feat: added CC Field #420
feat: added CC Field #420
Conversation
| ); | ||
| } | ||
|
|
||
| const Container = styled(FauxInput)` |
There was a problem hiding this comment.
Nit: would be nice to be consistent and always define variables/functions either at the top or bottom of the file.
There was a problem hiding this comment.
Yeah, I agree. I personally think that the file is easier to read if the main exported component is on top, and the rest on the bottom. But since we used the opposite convention in other files, I will follow that. Maybe we can revisit it later
| {/* Used to automatically resize the input based on the content */} | ||
| <InputMirror isBare aria-hidden="true" tabIndex={-1}> | ||
| {/* When the input is empty we show a space, otherwise the height of the mirror is less than the input */} | ||
| {inputValue || " "} |
There was a problem hiding this comment.
Nit: we've had issues with white space not existing/being supported in asian languages. Not sure if this case is even a problem but if it could be easily addressed with CSS that could be better.
There was a problem hiding this comment.
I'll give another try to see if it is fixable with CSS only
| case "cc_email": | ||
| return ( | ||
| <Suspense fallback={<></>}> | ||
| <CcField field={field} /> |
There was a problem hiding this comment.
At some point we might need some error handling when lazy loading, e.g. providing a regular text field that asks for emails separated by ,. In that case, it might be better to not lazy load here. No need to address now though.
There was a problem hiding this comment.
I am not sure if it makes sense to have a functional fallback component because we would need to support 2 different implementations. For example, with the regular text field, we would still need to parse the value and create some hidden inputs to submit the values.
There was a problem hiding this comment.
I agree it's maybe it's overkill for error handling 👍🏼
Let's see how large the js payloads will become. I'm thinking that in some extreme use case, e.g. Disney we might need to do some optimization and that may mean offering "smaller" components. Just something to keep in mind for later.
Fredx87
force-pushed
the
gianluca/cc-field-grid
branch
from
e23e273
to
3103fb8
Compare
Description
This PR adds a CC field to the new request form.
We decided to implement a custom component since Garden doesn't have a component that fits our use case.
In theory, it would be possible to use a Combobox, but the UX is not optimal since it doesn't make much sense to show a popup with suggested results when the user can input any value they want, and there isn't a set of predefined values.
The implementation resembles the existing component, and it is also similar to an example shown in the ARIA Authoring Practices Guide website, where the list of recipients in rendered in a grid. For this reason, I used the grid container from Garden.
I tested the component with VoiceOver on Safari and NVDA on Firefox, but we'll need to wait for an A11Y audit to refine it and ensure it is properly accessible.
Keyboard interaction
Screen reader
Other notes:
Screenshots
Screen.Recording.2023-08-22.at.10.54.07.mov
Checklist