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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/components/NcInputField/NcInputField.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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).

<!-- Label -->
Expand Down Expand Up @@ -132,9 +132,10 @@ export default {
props: {
/**
* The value of the input field
* If type is 'number' and a number is passed as value than the type of `update:value` will also be 'number'
*/
value: {
type: String,
type: [String, Number],
required: true,
},

Expand Down Expand Up @@ -331,7 +332,7 @@ export default {
},

handleInput(event) {
this.$emit('update:value', event.target.value)
this.$emit('update:value', this.type === 'number' && typeof this.value === 'number' ? parseFloat(event.target.value, 10) : event.target.value)
},

handleTrailingButtonClick(event) {
Expand Down
48 changes: 48 additions & 0 deletions tests/unit/components/NcInputField/NcInputField.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { shallowMount } from '@vue/test-utils'
import NcInputField from '../../../../src/components/NcInputField/index.js'

describe('NcInputField', () => {
it('should emit text when type is text', async () => {
const wrapper = shallowMount(NcInputField, {
propsData: {
label: 'label',
value: '',
type: 'text',
},
})

await wrapper.get('input').setValue('text')

expect(wrapper.emitted('update:value')).toEqual([['text']])
})

it('should emit text when type is number but value is text', () => {
const wrapper = shallowMount(NcInputField, {
propsData: {
value: '1',
type: 'number',
},
})

const input = wrapper.find('input').element as HTMLInputElement
input.value = '2'
input.dispatchEvent(new InputEvent('input'))

expect(wrapper.emitted('update:value')).toEqual([['2']])
})

it('should emit a number when type is number and value is number', () => {
const wrapper = shallowMount(NcInputField, {
propsData: {
value: 1,
type: 'number',
},
})

const input = wrapper.find('input').element as HTMLInputElement
input.value = '2'
input.dispatchEvent(new InputEvent('input'))

expect(wrapper.emitted('update:value')).toEqual([[2]])
})
})