fix: improve tiered pricing number input editing - #4536
Conversation
WalkthroughThe PR updates the Label component to explicitly render children and detect/style trailing asterisks as required field markers, and refactors tiered pricing numeric inputs to use a new DraftNumberInput component that maintains draft state during editing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/default/src/components/ui/label.tsx (1)
46-50: AlignLabelprops handling with repo TSX convention.This component now destructures props in the signature; the guideline requests using
props.xxxdirectly.As per coding guidelines: "Do not destructure component props unnecessarily; use
props.xxxdirectly for clarity and to avoid unnecessary code complexity."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/components/ui/label.tsx` around lines 46 - 50, The Label component should stop destructuring props in its parameter list; change the function signature to accept a single parameter named props of type React.ComponentProps<typeof LabelPrimitive.Root> and inside the body reference props.className, props.children and other props directly (instead of destructured variables), then pass {...props} to LabelPrimitive.Root and compose className using props.className; update any references to className/children in the Label function to use props.xxx.web/default/src/features/system-settings/models/tiered-pricing-editor.tsx (1)
325-333: Preferprops.xxxaccess over parameter destructuring in this component.This new component destructures props in the function signature, which conflicts with the repo convention for TSX components.
Suggested refactor
-function DraftNumberInput({ - value, - onValueChange, - selectZeroOnFocus = true, - onBlur, - onFocus, - onMouseUp, - ...props -}: DraftNumberInputProps) { - const [draft, setDraft] = useState(() => formatNumberDraft(value)) +function DraftNumberInput(props: DraftNumberInputProps) { + const { + value, + onValueChange, + selectZeroOnFocus = true, + onBlur, + onFocus, + onMouseUp, + ...inputProps + } = props + const [draft, setDraft] = useState(() => formatNumberDraft(value)) @@ - {...props} + {...inputProps}As per coding guidelines: "Do not destructure component props unnecessarily; use
props.xxxdirectly for clarity and to avoid unnecessary code complexity."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/system-settings/models/tiered-pricing-editor.tsx` around lines 325 - 333, The DraftNumberInput component currently destructures props in its function signature; change it to accept a single props parameter and replace all destructured references (value, onValueChange, selectZeroOnFocus, onBlur, onFocus, onMouseUp, etc.) with props.value, props.onValueChange, props.selectZeroOnFocus (defaulting props.selectZeroOnFocus = true where used), and so on; update the DraftNumberInput function signature and all internal usages to follow the repository convention of accessing props via props.xxx rather than parameter destructuring.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/default/src/components/ui/label.tsx`:
- Around line 7-17: The required-marker regex only matches a trailing space
before '*' so labels like "Email*" are missed; update requiredMarkerPattern in
this file to match an optional space before the asterisk (e.g., use /\s*\*$/)
and change renderRequiredMarker to remove the marker robustly (for example,
strip the trailing space+asterisk via replace or trimEnd()+slice) before
rendering the label text and the <span className='text-destructive'>*</span>;
refer to requiredMarkerPattern and renderRequiredMarker to locate and apply the
fix.
---
Nitpick comments:
In `@web/default/src/components/ui/label.tsx`:
- Around line 46-50: The Label component should stop destructuring props in its
parameter list; change the function signature to accept a single parameter named
props of type React.ComponentProps<typeof LabelPrimitive.Root> and inside the
body reference props.className, props.children and other props directly (instead
of destructured variables), then pass {...props} to LabelPrimitive.Root and
compose className using props.className; update any references to
className/children in the Label function to use props.xxx.
In `@web/default/src/features/system-settings/models/tiered-pricing-editor.tsx`:
- Around line 325-333: The DraftNumberInput component currently destructures
props in its function signature; change it to accept a single props parameter
and replace all destructured references (value, onValueChange,
selectZeroOnFocus, onBlur, onFocus, onMouseUp, etc.) with props.value,
props.onValueChange, props.selectZeroOnFocus (defaulting props.selectZeroOnFocus
= true where used), and so on; update the DraftNumberInput function signature
and all internal usages to follow the repository convention of accessing props
via props.xxx rather than parameter destructuring.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3823c448-9b5f-4c0c-995a-07333d84436f
📒 Files selected for processing (2)
web/default/src/components/ui/label.tsxweb/default/src/features/system-settings/models/tiered-pricing-editor.tsx
| const requiredMarkerPattern = /\s\*$/ | ||
|
|
||
| function renderRequiredMarker(text: string, key?: React.Key) { | ||
| if (!requiredMarkerPattern.test(text)) { | ||
| return text | ||
| } | ||
|
|
||
| return ( | ||
| <span key={key}> | ||
| {text.slice(0, -1)} | ||
| <span className='text-destructive'>*</span> |
There was a problem hiding this comment.
Required-marker detection misses Label* (no-space suffix).
The current regex only matches "<text> *". Labels like Email* won’t be styled.
Suggested fix
-const requiredMarkerPattern = /\s\*$/
+const requiredMarkerPattern = /\s*\*$/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/components/ui/label.tsx` around lines 7 - 17, The
required-marker regex only matches a trailing space before '*' so labels like
"Email*" are missed; update requiredMarkerPattern in this file to match an
optional space before the asterisk (e.g., use /\s*\*$/) and change
renderRequiredMarker to remove the marker robustly (for example, strip the
trailing space+asterisk via replace or trimEnd()+slice) before rendering the
label text and the <span className='text-destructive'>*</span>; refer to
requiredMarkerPattern and renderRequiredMarker to locate and apply the fix.
* fix: follow required marker styling convention * fix: improve tiered pricing number input editing
* fix: follow required marker styling convention * fix: improve tiered pricing number input editing
Important
📝 变更描述 / Description
(简述:做了什么?为什么这样改能生效?请基于你对代码逻辑的理解来写,避免粘贴未经整理的内容)
修复新版
defaultUI 中阶梯定价数字输入框的编辑体验问题。此前价格字段直接把输入值转换成 number 保存,导致输入框清空时空字符串会立刻被转换回 0,所以无法删除默认的 0。同时在默认值为 0 时直接输入数字,会变成类似 0111 的追加效果。
本次改动为阶梯定价编辑器的数字输入增加编辑态处理:输入过程中保留用户正在编辑的文本,允许临时清空;当字段为 0 且获得焦点时自动选中原值,使新输入可以直接替换默认 0。最终生成表达式时仍按原有逻辑把空值归一为 0,不改变计费语义。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告,以证明变更生效)
iShot_2026-04-29_16.29.46.mp4
Summary by CodeRabbit