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(Field): add min and max props #13068

Merged
merged 7 commits into from
Sep 8, 2024
Merged

Conversation

tuskermanshu
Copy link
Contributor

close #13058

Field 组件添加了max 和 min 属性,支持最大最小值约束

close youzan#13058
给Field 组件添加了max 和 min 属性,支持最大最小值约束
Adding Unit Tests
@codecov-commenter
Copy link

codecov-commenter commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.65%. Comparing base (867e612) to head (b6327ab).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
packages/vant/src/field/Field.tsx 60.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13068      +/-   ##
==========================================
- Coverage   89.66%   89.65%   -0.01%     
==========================================
  Files         257      257              
  Lines        6987     6992       +5     
  Branches     1724     1727       +3     
==========================================
+ Hits         6265     6269       +4     
+ Misses        382      380       -2     
- Partials      340      343       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chenjiahan
Copy link
Member

这种 min / max 的校验通常是通过表单校验处理的,可以提示用户正确的输入范围是什么。

如果组件在填写阶段,直接修改用户输入的内容,从用户体验上来说不是很好,提示不够明确

@tuskermanshu
Copy link
Contributor Author

这种 min / max 的校验通常是通过表单校验处理的,可以提示用户正确的输入范围是什么。

如果组件在填写阶段,直接修改用户输入的内容,从用户体验上来说不是很好,提示不够明确

确实,这种行为可能会让用户感到困惑,直接修改他们的输入,无法明确提示。

@CatsAndMice
Copy link
Contributor

CatsAndMice commented Aug 19, 2024

有点击加、减图标改变输入框数值的场景,如果不加以限制,而是在表单提交阶段限制,用户又要重新改变输入框内的值,对于用户来说要多次操作,不如在填写阶段帮用户限制掉。
参考组件:https://arco.design/vue/component/input-number#API

@chenjiahan
Copy link
Member

你描述的是 Stepper 组件,这个组件已经提供了 min max 属性
https://vant-ui.github.io/vant/#/en-US/stepper

@CatsAndMice
Copy link
Contributor

你描述的是 Stepper 组件,这个组件已经提供了 min max 属性 https://vant-ui.github.io/vant/#/en-US/stepper

确实,Stepper可以满足需要

@CatsAndMice
Copy link
Contributor

但我觉得min / max 的校验不都是通过表单校验处理的,我们给使用者两种方式表单提交检验、输入框填写检验 。方便使用方自己选择哪种方式

@tuskermanshu
Copy link
Contributor Author

但我觉得min / max 的校验不都是通过表单校验处理的,我们给使用者两种方式表单提交检验、输入框填写检验 。方便使用方自己选择哪种方式

如果只是单纯控制输入字段的范围,使用 maxmin 属性比添加表单和通过事件来控制更加方便。而且,原生 HTML5 输入元素也支持 maxmin 属性。然而,这种方法在用户友好提示方面可能有所欠缺。

const { value } = event.target as HTMLInputElement;

if (value === '') {
updateValue('');
Copy link
Member

Choose a reason for hiding this comment

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

这个 updateValue('') 的作用是什么呢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

当输入框的值为空时,确保组件的值也被更新为空字符串。如果不进行此校验,默认情况下失去焦点时会将值设置为 min,这对用户来说是非预期的效果。不过,我将空字符串的校验放入类型校验内部去判断。优化了逻辑分支。请帮忙cr,感谢

@@ -348,6 +348,8 @@ export default {
| type | 输入框类型, 支持原生 input 标签的所有 [type 属性](https://developer.mozilla.org/zh-CN/docs/Web/HTML/Element/input#%3Cinput%3E_types),额外支持了 `digit` 类型 | _FieldType_ | `text` |
| size | 大小,可选值为 `large` `normal` | _string_ | - |
| maxlength | 输入的最大字符数 | _number \| string_ | - |
| min | 设置允许的最小值 | \_number | - |
| max | 设置允许的最大值 | \_number | - |
Copy link
Member

Choose a reason for hiding this comment

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

可以说明下 min/max 仅在特定的 type 下生效

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

tuskermanshu and others added 2 commits August 24, 2024 11:14
Comment on lines 426 to 441
if (props.type === 'number' || props.type === 'digit') {
const numericValue = value === '' ? '' : Number(value);
const min = props.min !== undefined ? props.min : -Infinity;
const max = props.max !== undefined ? props.max : Infinity;

let adjustedValue = numericValue;
if (numericValue !== '' && numericValue < min) {
adjustedValue = min;
} else if (numericValue !== '' && numericValue > max) {
adjustedValue = max;
}

updateValue(adjustedValue.toString());
} else {
updateValue(value);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (props.type === 'number' || props.type === 'digit') {
const numericValue = value === '' ? '' : Number(value);
const min = props.min !== undefined ? props.min : -Infinity;
const max = props.max !== undefined ? props.max : Infinity;
let adjustedValue = numericValue;
if (numericValue !== '' && numericValue < min) {
adjustedValue = min;
} else if (numericValue !== '' && numericValue > max) {
adjustedValue = max;
}
updateValue(adjustedValue.toString());
} else {
updateValue(value);
}
if ((props.type === 'number' || props.type === 'digit') && value !== '') {
const adjustedValue = clamp(
+value,
props.min ?? -Infinity,
props.max ?? Infinity,
);
updateValue(adjustedValue.toString(), 'onBlur');
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. 整个逻辑应该可以再和上面的 updateValue 整合一下
  2. type 为 digit 时,需要考虑处理后的数字为正整数,这个优先级应该高于 min/max

Copy link
Contributor Author

@tuskermanshu tuskermanshu Aug 26, 2024

Choose a reason for hiding this comment

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

感谢 review

Copy link
Member

@chenjiahan chenjiahan left a comment

Choose a reason for hiding this comment

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

👍🏻

@chenjiahan chenjiahan merged commit f0d3786 into youzan:main Sep 8, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

【van-field】数字类型的输入框支持设置最大值、最小值
5 participants