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: add v-model support for input-like components #5418

Merged
merged 4 commits into from
Oct 29, 2024
Merged

feat: add v-model support for input-like components #5418

merged 4 commits into from
Oct 29, 2024

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

ShGKme
ShGKme reviewed Mar 17, 2024
View reviewed changes
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
@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 removed this from the 8.18.0 milestone Sep 12, 2024
@Antreesy Antreesy added this to the 8.19.0 milestone 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.

kyteinsky and others added 3 commits October 22, 2024 13:12
@kyteinsky @Antreesy
feat: add v-model to remaining input components
c3223b5 
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]>
@Antreesy
fix(NcTextArea): remove non-existing prop 'type' for textarea
4e6ba04 
Signed-off-by: Maksim Sukharev <[email protected]>
@Antreesy
chore(docs): add v-model examples for updated components (checked)
84bb554 
Signed-off-by: Maksim Sukharev <[email protected]>
@Antreesy
Copy link
Contributor

Let's add event here as well.

Added model for NcActionRadio

Added docs examples with v-model for all components from Breaking changes list:

  • checked:
    • NcActionCheckbox
    • NcActionRadio
    • NcCheckboxRadioSwitch
  • value:
    • NcActionInput
    • NcActionTextEditable
    • NcInputField
    • NcPasswordField
    • NcRichContenteditable
    • NcSettingsInputText
    • NcTextArea
    • NcTextField
  • already had v-model:
    • NcColorPicker
    • NcDateTimePicker
    • NcDateTimePickerNative
    • NcSelect
    • NcSelectTags
    • NcSettingsSelectGroup
    • NcTimezonePicker

@Antreesy Antreesy requested a review from ShGKme October 22, 2024 12:52
@Antreesy Antreesy changed the title feat(NcCheckboxRadioSwitch): add v-model support for checked feat: add v-model support for input-like components Oct 22, 2024
@Antreesy Antreesy marked this pull request as ready for review October 22, 2024 15:23
@Antreesy Antreesy requested a review from susnux October 22, 2024 15:23
@Antreesy Antreesy added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 22, 2024
@ShGKme ShGKme mentioned this pull request Oct 28, 2024
Merged
21 tasks
@Antreesy Antreesy merged commit 64081ab into master Oct 29, 2024
19 checks passed
@Antreesy Antreesy deleted the feat/v-model-checkbox-radio branch October 29, 2024 08:12
@Antreesy
Copy link
Contributor

/backport 4e6ba04 to next

@backportbot backportbot bot mentioned this pull request Oct 29, 2024
Merged
@Antreesy Antreesy mentioned this pull request Oct 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Antreesy Antreesy Antreesy left review comments

@ShGKme ShGKme ShGKme approved these changes

@susnux susnux susnux approved these changes

@raimund-schluessler raimund-schluessler Awaiting requested review from raimund-schluessler

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement New feature or request feature: checkbox-radio-switch Related to the checkbox-radio-switch component
Projects
None yet
Milestone
8.20.0
Development

Successfully merging this pull request may close these issues.
4 participants
@kyteinsky @ShGKme @Antreesy @susnux