-
Notifications
You must be signed in to change notification settings - Fork 31
feat: add optional description to combobox #2042
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
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds an optional description feature to the Combobox component that displays below the label and repositions error messages to appear between the description and input field for better visual hierarchy.
- Adds an optional
description
prop to the Combobox component - Moves error messages to appear above the input field and below the description
- Updates aria-describedby to properly reference both description and error elements
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/styles/forms.css | Adds flex layout styles for proper field alignment |
packages/react/src/components/Combobox/Combobox.tsx | Implements description prop and repositions error rendering |
packages/react/src/components/Combobox/Combobox.test.tsx | Adds test coverage for description functionality |
docs/pages/components/Combobox.mdx | Documents the new description feature with example |
Comments suppressed due to low confidence (1)
packages/react/src/components/Combobox/Combobox.test.tsx:271
- This test is duplicated by the test at line 1678. The tests have the same functionality but different naming - consider consolidating them to avoid redundancy.
test('should render combobox with both description and error', () => {
Co-authored-by: Copilot <[email protected]>
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking a look at this! This is definitely a good start, a few things to clean up but nothing big. We'll also need to ensure we have updated screenshots for this with the description. If you're unsure where to get started with that, let me know and we can pair!
Looking at the screenshots, the error and description do not look to have the correct spacing from the field |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor things. I think addressing the class name fixes in the component should address the differences that are being shown in the screenshot.
Co-authored-by: Jason <[email protected]>
Co-authored-by: Jason <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor thing. We also need to get a final @dequelabs/design-team signoff and we're good to go.
Co-authored-by: Jason <[email protected]>
Closes: #1657
This adds an optional attribute for a description below the Label of a combobox. It also moves the error to above the input and below the description. Finally, it adds a caution icon before the error message.