fix: inconsistent spacing around search form #5495
Conversation
littlesparklet
requested review from
anncwb,
vince292007 and
mynetfan
as code owners
|
WalkthroughThis pull request modifies the styling in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
mynetfan
changed the title
|
Could you please merge this simple change? Are there any other considerations that need to be taken into account? @mynetfan |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/@core/ui-kit/form-ui/src/use-form-context.ts (2)
52-60: Consider providing a numeric default (e.g., 0) or a configurable placeholder.
Right now, the fallback for aZodNumberrule isnull, which some users may find unintuitive. If you want to avoid displaying "0," consider making the default numeric placeholder configurable or providing guidance that the intended default is null.
67-98: Extend default value extraction coverage for other Zod types.
Currently,getCustomDefaultValuehandles strings, numbers, objects, and intersections. You might want to add support for optional types (ZodOptional), arrays (ZodArray), or unions (ZodUnion) if your schema defines them. You can gracefully handle those cases or at least flag them to avoid unexpected behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/@core/ui-kit/form-ui/src/use-form-context.ts(2 hunks)
🔇 Additional comments (1)
packages/@core/ui-kit/form-ui/src/use-form-context.ts (1)
13-13: Looks good!
No issues with the new imports. The required Zod types are now properly referenced.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/@core/ui-kit/form-ui/src/use-form-context.ts (5)
52-60: Translate comments to English for consistency.The logic is well-structured, but the Chinese comments should be translated to English to maintain consistency across the codebase.
Apply this diff to translate the comments:
- // 检查规则是否适合提取默认值 + // Check if the rule is suitable for default value extraction - // 否则将规则添加到 zodObject + // Otherwise add the rule to zodObject - // 如果有自定义默认值，直接使用 + // If there's a custom default value, use it directly
68-69: Add TypeScript type annotations for better type safety.The function declaration could benefit from more specific TypeScript types.
Apply this diff to add proper type annotations:
- function getCustomDefaultValue(rule: any): any { + function getCustomDefaultValue(rule: ZodString | ZodNumber | ZodObject<any> | ZodIntersection<any, any> | any): unknown {
72-74: Consider making number defaults configurable.Using
nullas a default for numbers might not be suitable for all cases. Consider making this configurable or usingundefinedto allow the form library to handle the default.Apply this diff to make the number default configurable:
- return null; // 默认为 null（避免显示 0） + return undefined; // Let the form library handle the default value
81-96: Add type guards for safer intersection handling.The intersection type handling could be more robust with proper type guards and error handling.
Consider applying this improvement:
} else if (rule instanceof ZodIntersection) { - // 对于交集类型，从schema 提取默认值 + // Extract default values from intersection schema const leftDefaultValue = getCustomDefaultValue(rule._def.left); const rightDefaultValue = getCustomDefaultValue(rule._def.right); - // 如果左右两边都能提取默认值，合并它们 + // If both sides have object default values, merge them if ( + leftDefaultValue !== null && + rightDefaultValue !== null && typeof leftDefaultValue === 'object' && typeof rightDefaultValue === 'object' ) { + // Ensure neither side is null before merging return { ...leftDefaultValue, ...rightDefaultValue }; } - // 否则优先使用左边的默认值 + // Prioritize left side default value, fallback to right side return leftDefaultValue ?? rightDefaultValue;
44-99: Consider extracting default value logic into a separate utility.The default value handling logic is becoming complex enough to warrant its own utility module. This would improve maintainability and make it easier to test and modify default value strategies.
Consider:
- Moving
getCustomDefaultValueto a separate utility file- Making the default values configurable through a strategy pattern
- Adding unit tests specifically for default value handling
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/@core/ui-kit/form-ui/src/use-form-context.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (1)
packages/@core/ui-kit/form-ui/src/use-form-context.ts (1)
13-13: LGTM!The new Zod type imports are correctly specified and align with their usage in the new functionality.
Description
related to the issue #5429Type of change
Please delete options that are not relevant.
Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit