fix: follow required marker styling convention - #4533
Merged
Calcium-Ion merged 1 commit intoApr 29, 2026
Conversation
Contributor
WalkthroughLabel now renders its Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-18: The required-marker regex is too narrow (only matches a
single space before '*') so update requiredMarkerPattern and the trimming logic
to accept zero-or-more whitespace (including non-breaking spaces) before the
asterisk; e.g. replace the pattern with /\s*\*$/ and change renderRequiredMarker
to derive the label text by removing that trailing whitespace+asterisk (e.g. via
text.replace(/\s*\*$/, '')) before rendering the visible '*' in a styled span;
keep the function name renderRequiredMarker and the visible span with className
'text-destructive'.
🪄 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: 634eab6b-88b4-45a4-ac6d-ba8ba8ddde98
📒 Files selected for processing (1)
web/default/src/components/ui/label.tsx
Comment on lines
+7
to
+18
| 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> | ||
| </span> |
Contributor
There was a problem hiding this comment.
Broaden the required-marker matcher.
/\s\*$/ only catches labels with exactly one whitespace character before *. Labels like Name*, labels using non-breaking spaces, or other formatted variants will still render an unstyled asterisk.
🔧 Suggested fix
-const requiredMarkerPattern = /\s\*$/
+const requiredMarkerPattern = /\s*\*$/
function renderRequiredMarker(text: string, key?: React.Key) {
if (!requiredMarkerPattern.test(text)) {
return text
}
return (
<span key={key}>
- {text.slice(0, -1)}
+ {text.replace(/\s*\*$/, '')}
<span className='text-destructive'>*</span>
</span>
)
}🤖 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 - 18, The
required-marker regex is too narrow (only matches a single space before '*') so
update requiredMarkerPattern and the trimming logic to accept zero-or-more
whitespace (including non-breaking spaces) before the asterisk; e.g. replace the
pattern with /\s*\*$/ and change renderRequiredMarker to derive the label text
by removing that trailing whitespace+asterisk (e.g. via text.replace(/\s*\*$/,
'')) before rendering the visible '*' in a styled span; keep the function name
renderRequiredMarker and the visible span with className 'text-destructive'.
Contributor
Author
|
@t0ng7u 麻烦 review 一下，thanks |
Member
|
可以的话弄一下gpg签名 |
Calcium-Ion
approved these changes
Apr 29, 2026
yyhhyyyyyy
force-pushed
the
fix/required-label-asterisk-color
branch
from
April 29, 2026 07:41
211b354 to
aee22a4
Compare
Contributor
Author
已签名 |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
web/default/src/components/ui/label.tsx (1)
7-17:⚠️ Potential issue | 🟡 MinorRequired-marker regex is still too narrow
The matcher still only catches labels with exactly one whitespace before
*. That leaves no-space variants (e.g.Name*) unstyled.Suggested fix
-const requiredMarkerPattern = /\s\*$/ +const requiredMarkerPattern = /\s*\*$/ function renderRequiredMarker(text: string, key?: React.Key) { if (!requiredMarkerPattern.test(text)) { return text } return ( <span key={key}> - {text.slice(0, -1)} + {text.replace(/\s*\*$/, '')} <span className='text-destructive'>*</span> </span> ) }🤖 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 labels with one space before the star; update requiredMarkerPattern to allow zero or more spaces (e.g. /\s*\*$/) and in renderRequiredMarker use that pattern to strip the trailing star and any surrounding whitespace (e.g. derive labelText = text.replace(requiredMarkerPattern, '') instead of text.slice(0, -1)) so labels like "Name*" and "Name *" both render with the styled asterisk; reference: requiredMarkerPattern and renderRequiredMarker.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@web/default/src/components/ui/label.tsx`:
- Around line 7-17: The required-marker regex only matches labels with one space
before the star; update requiredMarkerPattern to allow zero or more spaces (e.g.
/\s*\*$/) and in renderRequiredMarker use that pattern to strip the trailing
star and any surrounding whitespace (e.g. derive labelText =
text.replace(requiredMarkerPattern, '') instead of text.slice(0, -1)) so labels
like "Name*" and "Name *" both render with the styled asterisk; reference:
requiredMarkerPattern and renderRequiredMarker.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb263d90-bb91-4961-ad5b-1a561085d69c
📒 Files selected for processing (1)
web/default/src/components/ui/label.tsx
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
📝 变更描述 / Description
完善必填项
*的显示效果🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
未修改：
修改后：
Summary by CodeRabbit