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

[next] fix: unify modelValue naming #4990

Merged
merged 1 commit into from
Dec 26, 2023
Merged

[next] fix: unify modelValue naming #4990

merged 1 commit into from
Dec 26, 2023

Conversation

raimund-schluessler
Copy link
Contributor

☑️ Resolves

  • This unifies the usage of value / modelValue. Every value prop got renamed to modelValue to align with the new usage by vue 3. For some components we did this already, so here I migrated the rest to have it consistent.

@raimund-schluessler raimund-schluessler added bug Something isn't working 3. to review Waiting for reviews vue 3 Related to the vue 3 migration labels Dec 23, 2023
@raimund-schluessler raimund-schluessler added this to the 9.0.0 next Vue 3 milestone Dec 23, 2023
@raimund-schluessler raimund-schluessler changed the title fix: unify modelValue naming [next] fix: unify modelValue naming Dec 23, 2023
@raimund-schluessler raimund-schluessler changed the title [next] fix: unify modelValue naming [next] fix: unify modelValue naming Dec 23, 2023
@raimund-schluessler raimund-schluessler added enhancement New feature or request feature: actions Related to the actions components feature: settings Related to the settings component feature: checkbox-radio-switch Related to the checkbox-radio-switch component labels Dec 23, 2023
@raimund-schluessler raimund-schluessler marked this pull request as ready for review December 23, 2023 12:34
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, modelValue should be used only in a meaning of a model, controlled value of the component, not a name for value.

Then it would align with Vue's model (input[type=checkbox] has v-model for checked, not value) and meaning of v-model directive (there always would be update:modelValue for updating component's model).

@raimund-schluessler
Copy link
Contributor Author

@ShGKme Thanks for the insight. I didn't look at it this way yet, but I think it makes sense. I will adjust the PR accordingly.

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler
Copy link
Contributor Author

I adjusted the PR. It now only contains changes related to renaming value to modelValue, as they should be quick to review.
I will send another PR regarding the renaming of other variables such as checked in NcCheckboxRadioSwitch.

@skjnldsv skjnldsv merged commit 39cbca1 into next Dec 26, 2023
14 checks passed
@skjnldsv skjnldsv deleted the fix/noid/model-value branch December 26, 2023 07:51
@susnux susnux mentioned this pull request Jan 23, 2024
1 task
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 bug Something isn't working enhancement New feature or request feature: actions Related to the actions components feature: settings Related to the settings component vue 3 Related to the vue 3 migration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants