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(field-multiselect): setup syntax [khcp-11341] #1885

Merged
merged 6 commits into from
Jan 10, 2025
Merged

feat(field-multiselect): setup syntax [khcp-11341] #1885

merged 6 commits into from
Jan 10, 2025

Conversation

kaiarrowood
Copy link
Contributor

@kaiarrowood kaiarrowood commented Jan 9, 2025 •
edited
Loading

Summary

Convert KMultiselect to setup syntax for KHCP-11341.

Other fixes:

  • Add testing of style classes to other entities
  • add support for missing props to FieldMultiselect:
    • id
    • classNames
    • disabled
    • name
    • add data-testid to KPop

@kaiarrowood kaiarrowood self-assigned this Jan 9, 2025
@kaiarrowood kaiarrowood marked this pull request as ready for review January 9, 2025 16:52
Leopoldthecoder
Leopoldthecoder previously approved these changes Jan 10, 2025
View reviewed changes
Copy link
Member

@Leopoldthecoder Leopoldthecoder left a comment

Choose a reason for hiding this comment

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

Manually tested and LGTM

Justineo
Justineo reviewed Jan 10, 2025
View reviewed changes
Copy link
Contributor

@Justineo Justineo left a comment

Choose a reason for hiding this comment

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

Could you also describe any other changes made in addition to the refactoring to <script setup> in the PR?

packages/core/forms/src/components/fields/FieldMultiselect.vue Show resolved Hide resolved
packages/core/forms/src/components/fields/FieldMultiselect.vue Outdated Show resolved Hide resolved
packages/core/forms/src/components/fields/FieldMultiselect.vue Outdated Show resolved Hide resolved
packages/core/forms/src/components/fields/FieldMultiselect.vue Show resolved Hide resolved
@kaiarrowood @Justineo
fix: pr feedback
22be3b7 
Co-authored-by: GU Yiling <[email protected]>
@kaiarrowood kaiarrowood enabled auto-merge (squash) January 10, 2025 14:23
Justineo
Justineo previously approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@Justineo Justineo left a comment

Choose a reason for hiding this comment

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

LGTM!

Justineo
Justineo approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@Justineo Justineo left a comment

Choose a reason for hiding this comment

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

LGTM

@kaiarrowood kaiarrowood merged commit df4ffbb into main Jan 10, 2025
11 checks passed
@kaiarrowood kaiarrowood deleted the feat/field-multiselect-setup-syntax branch January 10, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Justineo Justineo Justineo approved these changes

@Leopoldthecoder Leopoldthecoder Leopoldthecoder left review comments

@LukeSwierlik LukeSwierlik Awaiting requested review from LukeSwierlik LukeSwierlik is a code owner

@QueaT-kong QueaT-kong Awaiting requested review from QueaT-kong QueaT-kong is a code owner

Assignees

@kaiarrowood kaiarrowood

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kaiarrowood @Justineo @Leopoldthecoder