Add support for configurable label placement in AutoFormFieldWrapper#1757
Add support for configurable label placement in AutoFormFieldWrapper#1757
Conversation
WalkthroughA single React component is updated to support configurable label placement through a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-ui/src/app/features/builder/block-properties/auto-form-field-wrapper.tsx (1)
155-166: Add explicit return type for exported function.The exported
AutoFormFieldWrapperfunction is missing an explicit return type, which is required per the coding guidelines for TypeScript.As per coding guidelines, use explicit return types for exported functions in TypeScript.
Apply this diff:
const AutoFormFieldWrapper = ({ placeBeforeLabelText = false, labelPlacement = 'top', children, hideDescription, allowDynamicValues, propertyName, inputName, property, disabled, field, -}: AutoFormFieldWrapperProps) => { +}: AutoFormFieldWrapperProps): JSX.Element => { const form = useFormContext();
🧹 Nitpick comments (2)
packages/react-ui/src/app/features/builder/block-properties/auto-form-field-wrapper.tsx (2)
133-153: Add explicit return type for consistency.While the component logic is sound and consolidates label rendering effectively, consider adding an explicit return type (
: JSX.Element) for better type safety and consistency with TypeScript best practices.Apply this diff:
const LabelContent = ({ property, placeBeforeLabelText, dynamicViewToggled, children, -}: LabelContentProps) => { +}: LabelContentProps): JSX.Element => { return (
265-276: Optimize empty wrapper div.When
labelPlacement === 'left', the wrapper div (lines 266-276) contains an empty inner div, which adds an unnecessary DOM element. Consider conditionally rendering the wrapper or restructuring to avoid empty elements.Apply this diff to remove the unnecessary wrapper:
- <div className="flex items-center justify-between"> - <div> - {labelPlacement === 'top' && ( - <LabelContent - property={property} - placeBeforeLabelText={placeBeforeLabelText} - dynamicViewToggled={dynamicViewToggled} - > - {children} - </LabelContent> - )} - </div> - + <div className="flex items-center justify-between"> + {labelPlacement === 'top' && ( + <LabelContent + property={property} + placeBeforeLabelText={placeBeforeLabelText} + dynamicViewToggled={dynamicViewToggled} + > + {children} + </LabelContent> + )} <FormLabelButton property={property}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react-ui/src/app/features/builder/block-properties/auto-form-field-wrapper.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Write clear, self-documenting code with descriptive variable and function names
Include comments for complex logic or non-obvious behavior
Require braces for all control blocks, even single-line statements
Use one space between keywords and parentheses (e.g.,if (condition) {)
UsecamelCasefor variable and function names
UsePascalCasefor class and type names
UseUPPER_SNAKE_CASEfor constant names
Use lowercase with hyphens for file names (e.g.,user-profile.ts)
Write comments explaining why something is done, not what (code should be self-explanatory)
UseTODO:prefix for actionable technical debt comments
Document public functions, classes, and modules
Preferconstoverletorvarin JavaScript/TypeScript
Prefer arrow functions for callbacks and functional components
Extract helper functions where possible
**/*.{ts,tsx,js,jsx}: Use braces for all control blocks, even single-line statements
Use one space between keywords and parentheses (e.g., 'if (condition) {')
Use camelCase for variable and function names
Use PascalCase for classes and types
Use UPPER_SNAKE_CASE for constants
Use lowercase with hyphens for file names (e.g., user-profile.ts)
Use 'TODO:' prefix for actionable technical debt comments
Prefer const over let or var in TypeScript/JavaScript
Prefer arrow functions for callbacks and functional components
Files:
packages/react-ui/src/app/features/builder/block-properties/auto-form-field-wrapper.tsx
**/*.{ts,tsx,js,jsx,sh}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 2 spaces for indentation in TypeScript/JavaScript and shell scripts
Use 2-space indentation for TypeScript/JavaScript and shell scripts
Files:
packages/react-ui/src/app/features/builder/block-properties/auto-form-field-wrapper.tsx
**/*.{ts,tsx,js,jsx,json,sh,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx,json,sh,md,yml,yaml}: Maintain line length of 100–120 characters
Remove trailing whitespace from all files
Ensure newline at end of file
Files:
packages/react-ui/src/app/features/builder/block-properties/auto-form-field-wrapper.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use types and interfaces where appropriate in TypeScript
Use explicit return types for exported functions in TypeScript
**/*.{ts,tsx}: Use types and interfaces where appropriate in TypeScript/JavaScript
Use explicit return types for exported functions in TypeScript
Files:
packages/react-ui/src/app/features/builder/block-properties/auto-form-field-wrapper.tsx
packages/react-ui/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/react-ui/**/*.{tsx,ts}: Use React 18 for frontend development
Use Zustand for state management
Use react-query v5 for data fetching
Use shadcn for UI components
Use Axios with the existing wrapper inapi.tsandqspackage for query strings
Follow React hooks best practices
Prefer small, composable React components
Files:
packages/react-ui/src/app/features/builder/block-properties/auto-form-field-wrapper.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{tsx,jsx}: Use React 18 for frontend development
Follow best practices for React hooks
Prefer small, composable components
Use cn utility to group Tailwind classnames in React components
Files:
packages/react-ui/src/app/features/builder/block-properties/auto-form-field-wrapper.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Engine Image for amd64
- GitHub Check: Build Engine Image for arm64
- GitHub Check: Build App Image for amd64
🔇 Additional comments (2)
packages/react-ui/src/app/features/builder/block-properties/auto-form-field-wrapper.tsx (2)
36-36: LGTM! Type definition follows best practices.The new
labelPlacementprop uses a clear TypeScript union type with string literals, is appropriately optional, and has a sensible default value for backward compatibility.
288-312: LGTM! Layout implementation is solid.The two-layout approach (top vs left label placement) is implemented correctly:
- Top placement: Label and controls in top row, input below
- Left placement: Label and input side-by-side with proper flex properties
The conditional rendering logic correctly handles all combinations of
dynamicViewToggledandplaceBeforeLabelText, and the CSS classes (shrink-0,flex-1,min-w-0) ensure proper layout behavior.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Pull request overview
This PR adds support for configurable label placement in the AutoFormFieldWrapper component, allowing labels to be positioned either above ('top') or to the left ('left') of form fields.
Key changes:
- Introduced a new
labelPlacementprop with 'top' and 'left' options - Extracted label rendering logic into a reusable
LabelContentcomponent - Restructured the layout to support both horizontal and vertical label placements
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ection select components (#1757)
Part of OPS-3164.
Summary by CodeRabbit
New Features
✏️ Tip: You can customize this high-level summary in your review settings.