Skip to content

Comments

feat!: split NcSelect super component into NcSelect and NcSelectUsers#6732

Merged
susnux merged 5 commits intomainfrom
feat/user-select
Apr 16, 2025
Merged

feat!: split NcSelect super component into NcSelect and NcSelectUsers#6732
susnux merged 5 commits intomainfrom
feat/user-select

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented Apr 6, 2025

☑️ Resolves

  1. Implement new NcSelectUsers (naming similar to NcSelectTags) component to split NcSelect super component
  2. Deprecate user-select prop of NcSelect
  3. Fully migrate NcSelectUsers to Typescript
  4. Remove user-select property and behavior from NcSelect

The first two commits would need to be backported to v8 for forward compatibility.

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 2️⃣ Backport bugfixes to stable8 for maintained Vue 2 version.

@susnux susnux added enhancement New feature or request 3. to review Waiting for reviews feature: select Related to the NcSelect* components labels Apr 6, 2025
@susnux susnux added this to the v9.0.0-rc.0 milestone Apr 6, 2025
@susnux susnux force-pushed the feat/user-select branch from fb761a4 to ee6ca71 Compare April 6, 2025 17:12
@susnux susnux requested a review from ShGKme April 6, 2025 17:12
@susnux susnux added the 💥 breaking PR that requires a new major version label Apr 14, 2025
Copy link
Contributor

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

I tested using the styleguide and it seems to work. However, the NcSelect examples seem to be broken but work on main. Is that related?

image

@susnux susnux force-pushed the feat/user-select branch from ee6ca71 to 0c1c4c1 Compare April 16, 2025 08:50
@susnux susnux force-pushed the feat/user-select branch from 0c1c4c1 to d8ca52c Compare April 16, 2025 09:23
@susnux susnux requested a review from st3iny April 16, 2025 09:23
@susnux
Copy link
Contributor Author

susnux commented Apr 16, 2025

I tested using the styleguide and it seems to work. However, the NcSelect examples seem to be broken but work on main. Is that related?

Yes it was, it is now fixed. Accidentally removed the default value.

Comment on lines +216 to +222
defineProps<{
/**
* `aria-label` for the clear input button.
*
* @default 'Clear selected'
*/
ariaLabelClearSelected?: string
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicating everything looks painful, but the only option is if we have one component in JS and another in TS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to migrate select to TS someday we can cleanup then 😔

Copy link
Contributor

Choose a reason for hiding this comment

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

Then we need the type from vue-select

@ShGKme ShGKme changed the title feat: split NcSelect super component into NcSelect and NcSelectUsers feat!: split NcSelect super component into NcSelect and NcSelectUsers Apr 16, 2025
@susnux susnux merged commit d81f990 into main Apr 16, 2025
29 checks passed
@susnux susnux deleted the feat/user-select branch April 16, 2025 12:20
@susnux
Copy link
Contributor Author

susnux commented Apr 16, 2025

/backport d0f8b9b 4d60ca0 to stable8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews 💥 breaking PR that requires a new major version enhancement New feature or request feature: select Related to the NcSelect* components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants