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.

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

Add MultiselectField component #225

Merged
merged 6 commits into from
Jul 24, 2023
Merged

Add MultiselectField component #225

merged 6 commits into from
Jul 24, 2023

Conversation

ynotdraw
Copy link
Contributor

@ynotdraw ynotdraw commented Jul 21, 2023

🚀 Description

This PR adds MultiselectField support / <Form::Fields::Multiselect.


🔬 How to Test


📸 Images/Videos of Functionality

Screen.Recording.2023-07-21.at.9.10.57.AM.mov

@ynotdraw ynotdraw self-assigned this Jul 21, 2023
@changeset-bot
Copy link

changeset-bot bot commented Jul 21, 2023

🦋 Changeset detected

Latest commit: 076d015

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@crowdstrike/ember-toucan-core Patch

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

@github-actions
Copy link
Contributor

github-actions bot commented Jul 21, 2023

Preview URLs

Env: preview
Docs: https://ce808dbf.ember-toucan-core.pages.dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was duplicated 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is mostly copy / paste from our other Field docs, except updated to reflect the Multiselect Component API

Copy link
Contributor Author

@ynotdraw ynotdraw Jul 21, 2023

Choose a reason for hiding this comment

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

These were stragglers from the previous PR, where they were incorrect now since we added :remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A few updates from the previous PR where I didn't update to :default or some typos/incorrect information after re-reviewing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be pretty familiar - essentially copy paste from the other *Field components, only swapping out the underlying control to the Multiselect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to above, this test is basically copy paste of *-field-test files, exercising only the necessities. Most of the complex testing of the mulitselect is handled in the control test, so this essentially just makes sure it wraps around a Field properly and exercises @selected and @onChange briefly.

@ynotdraw
Copy link
Contributor Author

Going to go ahead and merge to unblock @clintcs's work, but for the other reviewers - feel free to leave comments and I'll get them addressed in downstream PRs! Since we are protected by changesets, consumers can't actually use this code until we merge the release PR 🎉

@ynotdraw ynotdraw merged commit 97a3c70 into main Jul 24, 2023
14 checks passed
@ynotdraw ynotdraw deleted the add-multiselect-field branch July 24, 2023 13:40
@github-actions github-actions bot mentioned this pull request Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants