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

feat(NcCheckboxRadioSwitch): add v-model support for checked #5418

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kyteinsky
Copy link
Contributor

Can we get this in nc/vue 8.x or does it not matter since it was added in vue9 recently (#4994)? Would be a nice QOL improvement still.

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

I think, that would be good to add model to every component that has modelValue in the next version.

Just to make further migration to v9 easier for library users.

All these components from the Breaking Changes list where it doesn't conflict with the current implementation.

https://github.com/nextcloud-libraries/nextcloud-vue/blob/next/CHANGELOG.md

@susnux susnux added enhancement New feature or request 2. developing Work in progress feature: checkbox-radio-switch Related to the checkbox-radio-switch component labels Mar 20, 2024
@susnux susnux added this to the 8.12.0 milestone Mar 20, 2024
@ShGKme
Copy link
Contributor

ShGKme commented Apr 9, 2024

Hey @kyteinsky!

What do you think about adding v-model to all these components?

@kyteinsky
Copy link
Contributor Author

Hello @ShGKme , sorry I didn't find time for it recently. Yeah, looks like a sound reasoning.

Is it alright if I do that the next week?

@ShGKme
Copy link
Contributor

ShGKme commented Apr 10, 2024

Is it alright if I do that the next week?

Sure

@susnux susnux modified the milestones: 8.12.0, 8.13.0 May 19, 2024
This facilitates migration from Vue 2 to Vue Next (3) easier.
Also comes with the v-model advantages like the number modifier
(v-model.number) which is not possible with .sync prop modifier.

Signed-off-by: Anupam Kumar <[email protected]>
@kyteinsky kyteinsky force-pushed the feat/v-model-checkbox-radio branch from 1e5f927 to eb74cc9 Compare May 22, 2024 01:46
@kyteinsky
Copy link
Contributor Author

Some components did not need modification and already work with v-model:

  1. NcColorPicker
  2. NcDateTimePicker
  3. NcDateTimePickerNative
  4. NcSelect
  5. NcSelectTags
  6. NcSettingsSelectGroup
  7. NcTimezonePicker

NcActionRadio does not make use of v-model in any useful way so I left it out. Enlighten me if I'm wrong here.

@susnux susnux modified the milestones: 8.13.0, 8.14.0 Jun 25, 2024
@susnux susnux modified the milestones: 8.14.0, 8.15.0 Jul 4, 2024
@Antreesy Antreesy modified the milestones: 8.15.0, 8.15.1 Jul 23, 2024
@susnux susnux modified the milestones: 8.15.1, 8.15.2, 8.16.0 Jul 29, 2024
@Antreesy Antreesy modified the milestones: 8.16.0, 8.17.0 Aug 5, 2024
@susnux susnux modified the milestones: 8.17.0, 8.18.0 Aug 21, 2024
@susnux susnux marked this pull request as draft August 30, 2024 12:04
@Antreesy Antreesy modified the milestones: 8.18.0, 8.19.0 Sep 12, 2024
@susnux susnux modified the milestones: 8.19.0, 8.20.0 Sep 17, 2024
@ShGKme
Copy link
Contributor

ShGKme commented Oct 9, 2024

Some components did not need modification and already work with v-model:

Confirm, all of them have the default model with :value + @input

NcActionRadio does not make use of v-model in any useful way so I left it out. Enlighten me if I'm wrong here.

It is the same as NcActionCheckbox, it has checked with update:checked event. Let's add event here as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress enhancement New feature or request feature: checkbox-radio-switch Related to the checkbox-radio-switch component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants