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

enh(NcInputField): Support numeric values - if numeric also emit numeric #4926

Merged
merged 1 commit into from Jan 22, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Dec 6, 2023

☑️ Resolves

If the type is set to number and a number is passed as value also emit a number instead of a string.

It always bugs me that you have to write wrappers otherwise, example:

<template>
<NcInputField type="number" :value="someObject.number" @update:value="(v) => someObject.number = Number(v)" />
</template>

vs with this:

<template>
<NcInputField type="number" :value.sync="someObject.number" />
</template>

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@susnux susnux added enhancement New feature or request 3. to review Waiting for reviews feature: input-field Covering the InputField, TextField, ... labels Dec 6, 2023
@susnux susnux added this to the 8.4.0 milestone Dec 6, 2023
src/components/NcInputField/NcInputField.vue Outdated Show resolved Hide resolved
tests/unit/components/NcInputField/NcInputField.spec.ts Outdated Show resolved Hide resolved
src/components/NcInputField/NcInputField.vue Outdated Show resolved Hide resolved
tests/unit/components/NcInputField/NcInputField.spec.ts Outdated Show resolved Hide resolved
@ShGKme ShGKme modified the milestones: 8.4.0, 8.5.0 Dec 27, 2023
@susnux susnux force-pushed the enh/ncinputfield-support-numeric-value branch from b6d88ac to 68abfbb Compare January 21, 2024 22:22
@susnux susnux requested a review from ShGKme January 21, 2024 22:22
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.

Fine by me, but feels like reimplementing functionality that already exists in v9 because of Vue 3

If the type is set to number and a number is passed as value also emit a number instead of a string.

Co-authored-by: Grigorii K. Shartsev <me@shgk.me>
Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the enh/ncinputfield-support-numeric-value branch from 68abfbb to a41711a Compare January 22, 2024 03:05
@susnux
Copy link
Contributor Author

susnux commented Jan 22, 2024

Fine by me, but feels like reimplementing functionality that already exists in v9 because of Vue 3

Well then remove with v9 and vue 3 :)

@susnux susnux enabled auto-merge January 22, 2024 03:05
@susnux susnux merged commit 10a76bd into master Jan 22, 2024
15 checks passed
@susnux susnux deleted the enh/ncinputfield-support-numeric-value branch January 22, 2024 03:07
@Pytal Pytal mentioned this pull request Jan 23, 2024
@@ -53,7 +53,7 @@ For a list of all available props and attributes, please check the [HTMLInputEle
'input-field__input--error': error,
'input-field__input--pill': pill,
}]"
:value="value"
:value="value.toString()"
v-on="$listeners"
@input="handleInput">
Copy link
Contributor

@Antreesy Antreesy Jan 29, 2024

Choose a reason for hiding this comment

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

Just to raise awareness (as i spend quite a time to understand it 🙈):

  • TextField don't listen for @update:value, but for @input
  • As it wasn't modified by this PR, TextField always emits a string

Is it expected? As InputField is marked as internal component, we usually avoid to use it directly in the app
What should we do?

  • update vue-lib docs, and 'allow' to use InputField?
  • expand PR changes to TextField? (not sure about rest wrappers)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically a lot of apps use NcInputField because of the button limitations (NcTextField does not allow any kind of trailing button).

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 enhancement New feature or request feature: input-field Covering the InputField, TextField, ...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants