Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(FluidDropdown): implement FluidDropdown #12128

Merged
merged 10 commits into from
Oct 6, 2022
Merged

feat(FluidDropdown): implement FluidDropdown #12128

merged 10 commits into from
Oct 6, 2022

Conversation

tw15egan
Copy link
Member

@tw15egan tw15egan commented Sep 20, 2022 •
edited
Loading

Closes #12107

Adds in unstable_FluidDropdown

Changelog

New

  • unstable_FluidDropdown component
  • Styles for FluidDropdown (_fluid-dropdown.scss)
  • Stories for FluidDropdown (FluidDropdown.stories.js)
  • Tests for FluidDropdown

Changed

  • Added FormContext from FluidForm to Dropdown.js and Listbox.js to handle FluidDropdown variant

Testing / Reviewing

Go to unstable_FluidDropdown and test out each story.

  • Ensure all controls in the playground story work as expected
  • Ensure there are no a11y violations
  • Ensure all keyboard functionality is working

Go to Dropdown and ensure there have been no regressions

@netlify
Copy link

netlify bot commented Sep 20, 2022 •
edited
Loading

✅ Deploy Preview for carbon-components-react ready!

Name Link
🔨 Latest commit 90d0fe9
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/633ec9a771fd3200099ab9a1
😎 Deploy Preview https://deploy-preview-12128--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Sep 20, 2022 •
edited
Loading

✅ Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 90d0fe9
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/633ec9a7c53533000863aaa9
😎 Deploy Preview https://deploy-preview-12128--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@tw15egan tw15egan marked this pull request as ready for review September 21, 2022 17:25
@tw15egan tw15egan requested a review from a team as a code owner September 21, 2022 17:25
@tw15egan tw15egan requested a review from a team as a code owner September 21, 2022 17:50
@tw15egan
Copy link
Member Author

@tay1orjones here's the attempt at refactoring fluid-dropdown to just port in all styles from fluid-list-box. I'm pretty sure this will get us 90% of the way to FluidMultiSelect and FluidComboBox but wanted to get your thoughts

@tw15egan tw15egan mentioned this pull request Sep 26, 2022
Merged
@abbeyhrt
Copy link
Contributor

abbeyhrt commented Sep 27, 2022 •
edited
Loading

@tw15egan Do the styles for fluid-list-box and fluid dropdown need to be re-exported through @carbon/react? If so, I think you can add them to this: https://github.com/carbon-design-system/carbon/blob/main/packages/styles/files.js and run build-styles in package/react and the export should be generated. I think right now, the styles are available through the /components/index.scss but someone wouldn't be able to import just the styles for fluid-dropdown or fluid-list-box, if they wanted to

@tw15egan
Copy link
Member Author

@abbeyhrt great catch, completely forgot about that. Will add them in

@tw15egan
Copy link
Member Author

@abbeyhrt updated 👍🏻

aagonzales
aagonzales requested changes Sep 28, 2022
View reviewed changes
Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

Everything looks good except this one: The border-subtle divider in the error and warning states should only be 1px not 2px.

image

image

@tw15egan
Copy link
Member Author

@aagonzales whoops, meant to be border-bottom not border 😮‍💨

Should be good to go now 👍🏻

aagonzales
aagonzales approved these changes Sep 28, 2022
View reviewed changes
Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

Looks good to go!

@tw15egan
Copy link
Member Author

tw15egan commented Oct 3, 2022

@aledavila ready for review

@kodiakhq
Copy link
Contributor

kodiakhq bot commented Oct 5, 2022

This PR currently has a merge conflict. Please resolve this and then re-add the status: ready to merge 🎉 label.

@kodiakhq kodiakhq bot merged commit 5e9d42e into carbon-design-system:main Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aagonzales aagonzales aagonzales approved these changes

@aledavila aledavila aledavila approved these changes

@abbeyhrt abbeyhrt abbeyhrt approved these changes

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
Design System
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fluid inputs: FluidDropdown
4 participants
@tw15egan @abbeyhrt @aagonzales @aledavila