Thulasizwe/en/form components width #4022
Conversation
fileList
- Changed the `styles` prop to `style` in the FileUpload component for consistency with other components. - Updated the `IFileUploadProps` interface to use `CSSProperties` for the `style` prop. - Refactored the styles handling in the FileUpload component to accommodate new default styles. - Enhanced settings for the FileUpload component to include new properties for layout and gap configurations. - Migrated existing styles to a new structure for better organization and maintainability.
- Added missing commas in object literals for better readability and to prevent potential issues. - Removed unnecessary console.log statements to clean up the code. - Ensured consistent formatting of props and styles across various components. - Updated the `formItemMargin` property in the default form settings for clarity. - Enhanced the `getStyleValue` function signature for better type safety.
- Changed the parameter types in `getComponentTypeInfo`, `getComponentDimensions`, `getDeviceDimensions`, and `getDeviceFlexBasis` functions to use `IToolboxComponent` and `CSSProperties` for improved type safety. - Updated the `createRootContainerStyle` function to accept `CSSProperties` instead of `ComponentDimensions` for better consistency in styling utilities.
- Updated destructuring of `model` to provide a default empty object, preventing runtime errors when `model` is null or undefined.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds typed dimension parsing and form-item margin settings; introduces device-aware dimension/styling utilities and hooks; restructures file-upload and stored-files styling/migrators toward thumbnail-centric shapes; applies widespread sizing and style adjustments across designer components and global layout styles. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Designer Component
participant StylesHook as useFormComponentStyles / useStyles
participant TypeUtils as componentTypeUtils
participant DimUtils as dimensionUtils
participant Styler as stylingUtils
participant Renderer as Render Wrapper
Component->>StylesHook: request styles for model / thumbnail
activate StylesHook
StylesHook->>TypeUtils: getComponentTypeInfo(component)
TypeUtils-->>StylesHook: return typeInfo
StylesHook->>DimUtils: getComponentDimensions(typeInfo, dimensionsStyles)
DimUtils-->>StylesHook: return dimensions CSS
StylesHook->>Styler: createRootContainerStyle(dimensions, margins, isInput)
Styler-->>StylesHook: container style
StylesHook-->>Component: final styles + classNames (includes margins/dimensions)
deactivate StylesHook
Component->>Renderer: render using final styles / renderComponentModel
Renderer-->>Component: rendered output (wrapped, sized, draggable)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Files warranting extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
shesha-reactjs/src/designer-components/richTextEditor/index.tsx (2)
68-68: Incomplete dependency array may cause stale closures.The
useDeepCompareMemoKeepReferencedependency array at line 68 is missingformData, butformDatais used within the memoized config object at line 53 (style: getStyle(model?.style, formData)). This can lead to stale closures where changes toformDatadon't trigger recalculation of the config.Apply this diff to include
formDatain the dependency array:- }, [model, model.readOnly]); + }, [model, model.readOnly, formData]);
63-63: Fix missingformModeand stale dependency in config memoization.The file uses
formModeon line 63 but never imports or defines it. TheuseFormDatahook only provides{ data }, whileformModerequires theuseForm()hook. Additionally, the config computation on line 39 usesformData(line 53) but the dependency array on line 68 is missing it.Apply this fix:
-import { useFormData } from '@/providers'; +import { useForm, useFormData } from '@/providers';Then update the Factory component:
const { data: formData } = useFormData(); + const { formMode } = useForm(); const { allStyles } = model;Finally, update the dependency array to include
formData:- }, [model, model.readOnly]); + }, [model, model.readOnly, formData]);shesha-reactjs/src/components/fileUpload/index.tsx (1)
199-215: Invalid nested anchors; outer wraps inner .Nested anchors are invalid HTML and can break clicks/focus.
Apply:
- <a title={file.name}> + <div title={file.name}> <Space> <div className="thumbnail-item-name"> {(listType === 'text' || !hideFileName) && ( - <a + <a style={{ marginRight: '5px' }} onClick={isImageType(file.type) ? onPreview : () => downloadFile({ fileId: file.id, fileName: file.name })} > {listType !== 'thumbnail' && getFileIcon(file?.type)} {`${file.name} (${filesize(file?.size || 0)})`} </a> )} {showTextControls && fileControls(theme.application.primaryColor)} </div> </Space> - </a> + </div>shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx (1)
55-69: classNames misuse; avoid passing layout object as class.
classNames(className, styles.formItem, layout)adds keys likelabelCol/wrapperColas class names. Remove it and keep the object only in Form.Item props.- className: classNames(className, styles.formItem, layout), + className: classNames(className, styles.formItem), ... - labelCol: layout?.labelCol, - wrapperCol: hideLabel ? { span: 24 } : layout?.wrapperCol, + labelCol: colLayout?.labelCol, + wrapperCol: hideLabel ? { span: 24 } : colLayout?.wrapperCol,shesha-reactjs/src/designer-components/attachmentsEditor/settings.ts (1)
790-931: Complete the property rename refactor or add migration logic for backward compatibility.The rename from
container.*tothumbnail.*properties is incomplete. The reviewed file (attachmentsEditor/settings.ts) uses the newthumbnail.*naming, but the same properties indataList/settingsForm.ts(lines 951, 962, 972, 989, 1000, 1010, 1036, 1057) still reference the oldcontainer.*paths.No migration or translation logic was found in the codebase to handle the rename. Existing form configurations using
container.*will break.Action required: Either update all property references consistently across the codebase, or implement a data transformation layer that maps old
container.*paths to newthumbnail.*paths during load/save operations.
🧹 Nitpick comments (19)
shesha-reactjs/src/components/formDesigner/toolbar/debugButton.tsx (1)
11-21: Tighten state update and improve accessibility (toggle button).
- Use functional updater to avoid stale reads.
- Expose state to AT: add
aria-pressedand a dynamicaria-label.keyisn’t needed outside lists; remove to reduce noise.Apply this diff:
return ( - <Button - key="debug" + <Button onClick={() => { - setDebugMode(!isDebug); + setDebugMode(prev => !prev); }} icon={<BugOutlined />} - title="Debug" + title="Debug" + aria-pressed={isDebug} + aria-label={isDebug ? 'Disable debug mode' : 'Enable debug mode'} type="primary" size="small" ghost={!isDebug} /> );Optional: wrap with AntD Tooltip for a consistent hover hint instead of relying on the native
title.shesha-reactjs/src/designer-components/progress/index.tsx (1)
26-26: Remove unusedallStylesproperty.The
allStylesproperty is no longer used in the Factory function after the styling refactor. Consider removing it from the interface to keep the code clean.Apply this diff to remove the unused property:
- allStyles?: IFormComponentStyles;shesha-reactjs/src/designer-components/contextPropertyAutocomplete/styles.ts (1)
12-29: Consider consolidating with shared label styles.The
labelstyle here is nearly identical to the one in_settings/styles/styles.ts, with only the addition ofmargin-right: 8pxon the tooltip (line 22). If this margin adjustment is needed across multiple components, consider updating the shared style. Otherwise, this duplication may be acceptable as part of the component isolation strategy.shesha-reactjs/src/components/sidebarContainer/styles/styles.ts (1)
33-33: Consider cross-browser compatibility for height property.The use of
-webkit-fill-available(line 33) is vendor-specific. Consider adding a fallback or using a cross-browser alternative likeheight: 100%or CSS logical properties.> div { - height: -webkit-fill-available; + height: 100%; .ant-spin-nested-loading {shesha-reactjs/src/designer-components/_settings/utils/dimensions/utils.tsx (2)
8-11: Avoid nested/needless calc() and type canvasWidthDouble-wrapping values like calc(calc(...)) is noisy and can confuse devtools. Also, canvasWidth is implicitly any.
Suggested change:
-const getDimension = (main: string | number, canvasWidth?): string | number => { - const width = canvasWidth ? widthRelativeToCanvas(main, canvasWidth) : main; - return !hasNumber(main) ? main : `calc(${addPx(width)})`; +const getDimension = (main: string | number, canvasWidth?: string): string | number => { + const resolved = canvasWidth ? widthRelativeToCanvas(main, canvasWidth) : main; + if (!hasNumber(main)) return resolved; // e.g. 'auto' + const withUnit = addPx(resolved); + // If widthRelativeToCanvas already returned a calc(...) or unit value, just return it + return typeof withUnit === 'string' ? withUnit : String(withUnit); }
13-33: Don’t drop zero widths/heights; use explicit null/undefined checksTruthiness checks skip valid 0 values.
Suggested change:
export const getDimensionsStyle = (dimensions: IDimensionsValue, canvasWidth?: string): CSSProperties => { return { - width: dimensions?.width + width: dimensions && dimensions.width !== undefined ? getDimension(dimensions.width, canvasWidth) : undefined, - height: dimensions?.height + height: dimensions && dimensions.height !== undefined ? getDimension(dimensions.height, canvasWidth) : undefined, - minWidth: dimensions?.minWidth + minWidth: dimensions && dimensions.minWidth !== undefined ? getDimension(dimensions.minWidth, canvasWidth) : undefined, - minHeight: dimensions?.minHeight + minHeight: dimensions && dimensions.minHeight !== undefined ? getDimension(dimensions.minHeight, canvasWidth) : undefined, - maxWidth: dimensions?.maxWidth + maxWidth: dimensions && dimensions.maxWidth !== undefined ? getDimension(dimensions.maxWidth, canvasWidth) : undefined, - maxHeight: dimensions?.maxHeight + maxHeight: dimensions && dimensions.maxHeight !== undefined ? getDimension(dimensions.maxHeight, canvasWidth) : undefined, }; };shesha-reactjs/src/designer-components/checkbox/utils.ts (1)
16-55: Defaults look consistent; minor nit: explicit units commentLooks good and consistent with other components’ defaults. Consider adding a brief comment that numeric margins/sizes elsewhere are px by convention for consistency.
shesha-reactjs/src/providers/form/models.ts (1)
313-318: Document units for IFormItemMarginAdd a short JSDoc that these numbers are pixels (converted via addPx downstream) to avoid ambiguity.
-export interface IFormItemMargin { +/** Margin values in px (numbers converted via addPx downstream) */ +export interface IFormItemMargin { top?: number; bottom?: number; left?: number; right?: number; }shesha-reactjs/src/designer-components/textField/textField.tsx (1)
61-61: LGTM with a minor style nitpick.The explicit width and height sizing ensures the input component takes full available space, which aligns well with the PR's objectives.
For consistency, consider using the same quote style for both dimensions:
- style: { ...model.allStyles.fullStyle, width: '100%', height: "100%" }, + style: { ...model.allStyles.fullStyle, width: '100%', height: '100%' },shesha-reactjs/src/components/fileUpload/index.tsx (4)
139-170: Accessibility: anchors used as buttons; add semantics or use Button.Clickable
<a>without href harms keyboard/a11y. Prefer<Button type="link">(already imported) or add role/tabIndex and key handlers.Example fix for controls:
- <a onClick={onReplaceClick} style={{ color: color }}> - <SyncOutlined title="Replace" /> - </a> + <Button type="link" onClick={onReplaceClick} style={{ color }} icon={<SyncOutlined title="Replace" />} /> - <a onClick={(e) => onDeleteClick(e)} style={{ color: color }}> - <DeleteOutlined title="Remove" /> - </a> + <Button type="link" onClick={onDeleteClick} style={{ color }} icon={<DeleteOutlined title="Remove" />} /> - <a onClick={onPreview} style={{ color: color }}> - <EyeOutlined title="Preview" /> - </a> + <Button type="link" onClick={onPreview} style={{ color }} icon={<EyeOutlined title="Preview" />} /> - <a - onClick={() => downloadFile({ fileId: fileInfo?.id, fileName: fileInfo?.name })} - style={{ color: color }} - > - <DownloadOutlined title="Download" /> - </a> + <Button type="link" onClick={() => downloadFile({ fileId: fileInfo?.id, fileName: fileInfo?.name })} style={{ color }} icon={<DownloadOutlined title="Download" />} />And for the file name link (lines 203-209), consider
<Button type="link">similarly if it’s purely an action.Also applies to: 203-209
83-91: Prevent memory leaks: revoke object URL and abort inflight fetch.Revoke the created URL and abort fetch on unmount or when deps change.
- useEffect(() => { - if (fileInfo && url) { - fetch(url, { headers: { ...httpHeaders, 'Content-Type': 'application/octet-stream' } }) - .then((response) => response.blob()) - .then((blob) => URL.createObjectURL(blob)) - .then((url) => setImageUrl(url)); - } - }, [fileInfo]); + useEffect(() => { + if (!fileInfo || !url) return; + const ac = new AbortController(); + let objectUrl: string | undefined; + fetch(url, { headers: { ...httpHeaders, 'Content-Type': 'application/octet-stream' }, signal: ac.signal }) + .then((response) => response.blob()) + .then((blob) => { + objectUrl = URL.createObjectURL(blob); + setImageUrl(objectUrl); + }) + .catch(() => {/* ignore abort/errors for preview */}); + return () => { + if (objectUrl) URL.revokeObjectURL(objectUrl); + ac.abort(); + }; + }, [fileInfo, url, httpHeaders]);
10-11: Deep imports from antd; prefer top-level types/components.
import { Image } from 'antd/lib'andUploadPropsfrom deep path are fragile. Useimport { Image, UploadProps } from 'antd'.-import { Image } from 'antd/lib'; -import { UploadProps } from 'antd/lib/upload/Upload'; +import { Image, UploadProps } from 'antd';
35-37: Stale props? thumbnailWidth/thumbnailHeight appear unused.If superseded by the new
style-driven approach, consider deprecating/removing to avoid confusion.shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (1)
14-46: Consider extracting getDimensionValue as a module-level helper.The
getDimensionValuefunction (lines 30-33) is currently defined insidegetComponentDimensions. Since it's a pure function that only depends on its parameters andisDataTableContext, consider extracting it to the module level for:
- Potential reuse in other functions
- Easier unit testing
- Improved readability
+const getDimensionValue = ( + dimensionType: keyof DimensionConfig, + dimensionsStyles: CSSProperties, + isDataTableContext: boolean +): string | number => { + if (isDataTableContext) return '100%'; + return dimensionsStyles?.[dimensionType]; +}; + export const getComponentDimensions = ( typeInfo: ComponentTypeInfo, dimensionsStyles: CSSProperties, ): CSSProperties => { const { isDataTableContext, isInput } = typeInfo; const width = isDataTableContext ? '100%' : dimensionsStyles?.width || 'auto'; const height = isDataTableContext ? '100%' : isInput ? 'auto' : dimensionsStyles?.height; - const getDimensionValue = (dimensionType: keyof DimensionConfig): string | number => { - if (isDataTableContext) return '100%'; - return dimensionsStyles?.[dimensionType]; - }; const flexBasis = dimensionsStyles?.maxWidth || dimensionsStyles?.width; return { width, height, - maxWidth: getDimensionValue('maxWidth'), - minWidth: getDimensionValue('minWidth'), - maxHeight: getDimensionValue('maxHeight'), - minHeight: getDimensionValue('minHeight'), + maxWidth: getDimensionValue('maxWidth', dimensionsStyles, isDataTableContext), + minWidth: getDimensionValue('minWidth', dimensionsStyles, isDataTableContext), + maxHeight: getDimensionValue('maxHeight', dimensionsStyles, isDataTableContext), + minHeight: getDimensionValue('minHeight', dimensionsStyles, isDataTableContext), flexBasis, }; };shesha-reactjs/src/designer-components/fileUpload/settingsForm.ts (5)
6-6: Avoid deep import from antd; use public typesDeep path 'antd/es/form/Form' is brittle. Import types from 'antd' instead.
-import { FormLayout } from 'antd/es/form/Form'; +import type { FormProps } from 'antd'; @@ - layout: 'vertical' as FormLayout, + layout: 'vertical' as FormProps['layout'],
257-258: Hide or rename user-visible label “Property router1”This looks like a dev label. Either hide it or give a meaningful title.
- label: 'Property router1', + label: 'Appearance', + // or hide it entirely: + // hideLabel: true,
263-265: Nit: clean up stray indentation in code stringLeading spaces inside _code are inconsistent with other entries.
- _code: " return contexts.canvasContext?.designerDevice || 'desktop';", + _code: "return contexts.canvasContext?.designerDevice || 'desktop';",
453-458: Minor: label text mismatchLabel says “Margin Padding” while the panel is “Margin & Padding”. Keep consistent (even if hidden).
- label: 'Margin Padding', + label: 'Margin & Padding',
334-348: Verify font weight option labels (likely incorrect)fontWeightsOptions currently have label: 'sectionSeparator' for all entries (see utils file). That will surface odd labels in this dropdown.
Proposed fix in shesha-reactjs/src/designer-components/_settings/utils/font/utils.tsx:
export const fontWeightsOptions = [ - { value: '100', label: 'sectionSeparator' }, - { value: '400', label: 'sectionSeparator' }, - { value: '500', label: 'sectionSeparator' }, - { value: '700', label: 'sectionSeparator' }, - { value: '900', label: 'sectionSeparator' }, + { value: '100', label: 'Thin' }, + { value: '400', label: 'Regular' }, + { value: '500', label: 'Medium' }, + { value: '700', label: 'Bold' }, + { value: '900', label: 'Black' }, ];Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (66)
shesha-reactjs/src/components/configurableForm/configurableFormRenderer.tsx(1 hunks)shesha-reactjs/src/components/configurableForm/models.ts(2 hunks)shesha-reactjs/src/components/entityPicker/styles/styles.ts(1 hunks)shesha-reactjs/src/components/fileUpload/index.tsx(5 hunks)shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx(4 hunks)shesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsx(2 hunks)shesha-reactjs/src/components/formDesigner/components/styles.ts(1 hunks)shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx(7 hunks)shesha-reactjs/src/components/formDesigner/styles/styles.ts(1 hunks)shesha-reactjs/src/components/formDesigner/toolbar/debugButton.tsx(1 hunks)shesha-reactjs/src/components/formDesigner/toolbar/saveMenu.tsx(1 hunks)shesha-reactjs/src/components/formDesigner/utils/componentTypeUtils.ts(1 hunks)shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts(1 hunks)shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts(1 hunks)shesha-reactjs/src/components/googlePlacesAutocomplete/styles/styles.ts(1 hunks)shesha-reactjs/src/components/keyInformationBar/index.tsx(1 hunks)shesha-reactjs/src/components/mainLayout/styles/indexStyles.ts(1 hunks)shesha-reactjs/src/components/panel/index.tsx(1 hunks)shesha-reactjs/src/components/panel/styles/styles.ts(3 hunks)shesha-reactjs/src/components/reactTable/index.tsx(1 hunks)shesha-reactjs/src/components/sidebarContainer/styles/styles.ts(1 hunks)shesha-reactjs/src/components/storedFilesRendererBase/index.tsx(2 hunks)shesha-reactjs/src/configuration-studio/document-definitions/form/toolbar.tsx(2 hunks)shesha-reactjs/src/designer-components/_settings/styles/styles.ts(3 hunks)shesha-reactjs/src/designer-components/_settings/utils/background/utils.tsx(2 hunks)shesha-reactjs/src/designer-components/_settings/utils/dimensions/utils.tsx(1 hunks)shesha-reactjs/src/designer-components/attachmentsEditor/attachmentsEditor.tsx(1 hunks)shesha-reactjs/src/designer-components/attachmentsEditor/settings.ts(12 hunks)shesha-reactjs/src/designer-components/attachmentsEditor/utils.ts(1 hunks)shesha-reactjs/src/designer-components/button/buttonGroup/buttonGroupComponent.tsx(1 hunks)shesha-reactjs/src/designer-components/button/util.ts(1 hunks)shesha-reactjs/src/designer-components/card/index.tsx(1 hunks)shesha-reactjs/src/designer-components/card/utils.ts(1 hunks)shesha-reactjs/src/designer-components/checkbox/checkbox.tsx(4 hunks)shesha-reactjs/src/designer-components/checkbox/interfaces.ts(1 hunks)shesha-reactjs/src/designer-components/checkbox/settingsForm.ts(4 hunks)shesha-reactjs/src/designer-components/checkbox/styles.ts(2 hunks)shesha-reactjs/src/designer-components/checkbox/utils.ts(1 hunks)shesha-reactjs/src/designer-components/collapsiblePanel/collapsiblePanelComponent.tsx(1 hunks)shesha-reactjs/src/designer-components/collapsiblePanel/utils.ts(1 hunks)shesha-reactjs/src/designer-components/container/data.ts(1 hunks)shesha-reactjs/src/designer-components/contextPropertyAutocomplete/index.tsx(3 hunks)shesha-reactjs/src/designer-components/contextPropertyAutocomplete/styles.ts(1 hunks)shesha-reactjs/src/designer-components/dateField/datePickerWrapper.tsx(1 hunks)shesha-reactjs/src/designer-components/dropdown/index.tsx(0 hunks)shesha-reactjs/src/designer-components/entityReference/entityReference.tsx(1 hunks)shesha-reactjs/src/designer-components/fileUpload/index.tsx(5 hunks)shesha-reactjs/src/designer-components/fileUpload/settings.ts(1 hunks)shesha-reactjs/src/designer-components/fileUpload/settingsForm.ts(3 hunks)shesha-reactjs/src/designer-components/fileUpload/utils.ts(1 hunks)shesha-reactjs/src/designer-components/inputComponent/styles.ts(1 hunks)shesha-reactjs/src/designer-components/numberField/numberField.tsx(1 hunks)shesha-reactjs/src/designer-components/progress/index.tsx(1 hunks)shesha-reactjs/src/designer-components/propertiesTabs/style.ts(2 hunks)shesha-reactjs/src/designer-components/radio/radio.tsx(1 hunks)shesha-reactjs/src/designer-components/richTextEditor/index.tsx(1 hunks)shesha-reactjs/src/designer-components/sectionSeprator/index.tsx(1 hunks)shesha-reactjs/src/designer-components/settingsInput/settingsInput.tsx(1 hunks)shesha-reactjs/src/designer-components/styleBox/components/input.tsx(3 hunks)shesha-reactjs/src/designer-components/styleBox/components/utils.ts(1 hunks)shesha-reactjs/src/designer-components/switch/switch.tsx(2 hunks)shesha-reactjs/src/designer-components/tabs/styles.ts(2 hunks)shesha-reactjs/src/designer-components/textArea/textArea.tsx(1 hunks)shesha-reactjs/src/designer-components/textField/textField.tsx(1 hunks)shesha-reactjs/src/hooks/formComponentHooks.ts(3 hunks)shesha-reactjs/src/providers/form/models.ts(3 hunks)
💤 Files with no reviewable changes (1)
- shesha-reactjs/src/designer-components/dropdown/index.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
PR: shesha-io/shesha-framework#3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.
Applied to files:
shesha-reactjs/src/designer-components/card/utils.tsshesha-reactjs/src/designer-components/card/index.tsxshesha-reactjs/src/designer-components/checkbox/checkbox.tsx
📚 Learning: 2025-05-28T07:55:21.036Z
Learnt from: teboho
PR: shesha-io/shesha-framework#3312
File: shesha-reactjs/src/hooks/formComponentHooks.ts:183-192
Timestamp: 2025-05-28T07:55:21.036Z
Learning: In shesha-reactjs formComponentHooks.ts, the background style initialization deliberately sets valid values from properties panel immediately to provide better UX, rather than waiting for the useDeepCompareEffect to run. This intentional pattern prevents a flash of no background and shows the background image immediately when the component renders.
Applied to files:
shesha-reactjs/src/hooks/formComponentHooks.ts
🧬 Code graph analysis (32)
shesha-reactjs/src/components/formDesigner/utils/componentTypeUtils.ts (2)
shesha-reactjs/src/interfaces/formDesigner.ts (1)
IToolboxComponent(68-179)shesha-reactjs/src/providers/form/models.ts (1)
IConfigurableFormComponent(210-285)
shesha-reactjs/src/components/keyInformationBar/index.tsx (1)
shesha-reactjs/src/designer-components/_settings/utils/dimensions/utils.tsx (1)
getDimensionsStyle(13-34)
shesha-reactjs/src/designer-components/entityReference/entityReference.tsx (1)
shesha-reactjs/src/components/entityReference/index.tsx (1)
EntityReference(79-315)
shesha-reactjs/src/configuration-studio/document-definitions/form/toolbar.tsx (1)
shesha-reactjs/src/components/formDesigner/toolbar/customActions.tsx (1)
CustomActions(12-55)
shesha-reactjs/src/designer-components/sectionSeprator/index.tsx (1)
shesha-reactjs/src/designer-components/_settings/utils/dimensions/utils.tsx (1)
getDimensionsStyle(13-34)
shesha-reactjs/src/designer-components/card/utils.ts (2)
shesha-reactjs/src/designer-components/fileUpload/utils.ts (1)
defaultStyles(3-39)shesha-reactjs/src/providers/form/models.ts (1)
IStyleType(70-85)
shesha-reactjs/src/designer-components/button/buttonGroup/buttonGroupComponent.tsx (1)
shesha-reactjs/src/designer-components/button/buttonGroup/buttonGroup.tsx (1)
ButtonGroup(248-261)
shesha-reactjs/src/designer-components/attachmentsEditor/settings.ts (1)
shesha-reactjs/src/interfaces/toolbarSettings.ts (1)
DesignerToolbarSettings(155-408)
shesha-reactjs/src/designer-components/contextPropertyAutocomplete/styles.ts (1)
shesha-reactjs/src/designer-components/_settings/styles/styles.ts (1)
useStyles(3-89)
shesha-reactjs/src/hooks/formComponentHooks.ts (1)
shesha-reactjs/src/designer-components/_settings/utils/dimensions/utils.tsx (1)
getDimensionsStyle(13-34)
shesha-reactjs/src/designer-components/attachmentsEditor/utils.ts (1)
shesha-reactjs/src/providers/form/models.ts (1)
IStyleType(70-85)
shesha-reactjs/src/designer-components/fileUpload/utils.ts (1)
shesha-reactjs/src/providers/form/models.ts (1)
IStyleType(70-85)
shesha-reactjs/src/designer-components/checkbox/utils.ts (2)
shesha-reactjs/src/providers/form/models.ts (1)
IStyleType(70-85)shesha-reactjs/src/designer-components/fileUpload/utils.ts (1)
defaultStyles(3-39)
shesha-reactjs/src/designer-components/_settings/utils/dimensions/utils.tsx (2)
shesha-reactjs/src/providers/canvas/utils.ts (1)
widthRelativeToCanvas(38-55)shesha-reactjs/src/utils/style.ts (2)
hasNumber(11-11)addPx(3-9)
shesha-reactjs/src/designer-components/checkbox/interfaces.ts (1)
shesha-reactjs/src/providers/form/models.ts (2)
IConfigurableFormComponent(210-285)IStyleType(70-85)
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx (2)
shesha-reactjs/src/providers/form/store/shaFormInstance.tsx (1)
settings(241-243)shesha-reactjs/src/components/formDesigner/components/styles.ts (1)
useStyles(3-37)
shesha-reactjs/src/components/configurableForm/models.ts (1)
shesha-reactjs/src/providers/form/models.ts (1)
IFormItemMargin(313-318)
shesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsx (1)
shesha-reactjs/src/components/formDesigner/components/styles.ts (1)
useStyles(3-37)
shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (1)
shesha-reactjs/src/components/formDesigner/utils/componentTypeUtils.ts (1)
ComponentTypeInfo(3-6)
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx (5)
shesha-reactjs/src/providers/form/store/shaFormInstance.tsx (2)
formSettings(150-152)formMode(154-156)shesha-reactjs/src/components/formDesigner/utils/componentTypeUtils.ts (1)
getComponentTypeInfo(8-16)shesha-reactjs/src/hooks/formComponentHooks.ts (1)
useFormComponentStyles(179-254)shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (3)
getComponentDimensions(14-46)getDeviceDimensions(48-59)getDeviceFlexBasis(61-65)shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (1)
createRootContainerStyle(16-51)
shesha-reactjs/src/components/formDesigner/components/styles.ts (1)
shesha-reactjs/src/components/formDesigner/styles/styles.ts (1)
useStyles(50-52)
shesha-reactjs/src/designer-components/fileUpload/settingsForm.ts (4)
shesha-reactjs/src/interfaces/toolbarSettings.ts (1)
DesignerToolbarSettings(155-408)shesha-reactjs/src/designer-components/_settings/utils/font/utils.tsx (3)
fontTypes(39-70)fontWeightsOptions(73-79)textAlignOptions(81-85)shesha-reactjs/src/designer-components/_settings/utils/border/utils.tsx (2)
getBorderInputs(136-242)getCornerInputs(251-303)shesha-reactjs/src/designer-components/_settings/utils/background/utils.tsx (3)
sizeOptions(106-106)positionOptions(108-117)repeatOptions(99-104)
shesha-reactjs/src/designer-components/styleBox/components/input.tsx (2)
shesha-reactjs/src/providers/form/store/shaFormInstance.tsx (1)
settings(241-243)shesha-reactjs/src/designer-components/styleBox/components/utils.ts (1)
getStyleValue(19-22)
shesha-reactjs/src/components/configurableForm/configurableFormRenderer.tsx (1)
shesha-reactjs/src/providers/form/store/shaFormInstance.tsx (1)
formSettings(150-152)
shesha-reactjs/src/designer-components/card/index.tsx (1)
shesha-reactjs/src/designer-components/card/utils.ts (1)
defaultStyles(3-33)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (2)
shesha-reactjs/src/hooks/formComponentHooks.ts (1)
useFormComponentStyles(179-254)shesha-reactjs/src/utils/style.ts (1)
addPx(3-9)
shesha-reactjs/src/designer-components/fileUpload/index.tsx (5)
shesha-reactjs/src/providers/form/models.ts (1)
IStyleType(70-85)shesha-reactjs/src/hooks/formComponentHooks.ts (1)
useFormComponentStyles(179-254)shesha-reactjs/src/components/fileUpload/index.tsx (1)
IFileUploadProps(24-41)shesha-reactjs/src/designer-components/fileUpload/utils.ts (2)
defaultStyles(3-39)containerDefaultStyles(41-58)shesha-reactjs/src/designer-components/_common-migrations/migrateStyles.ts (1)
migratePrevStyles(100-110)
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (2)
shesha-reactjs/src/utils/style.ts (2)
addPx(3-9)hasNumber(11-11)shesha-reactjs/src/components/formDesigner/utils/componentTypeUtils.ts (1)
ComponentTypeInfo(3-6)
shesha-reactjs/src/designer-components/_settings/styles/styles.ts (1)
shesha-reactjs/src/designer-components/contextPropertyAutocomplete/styles.ts (1)
useStyles(3-36)
shesha-reactjs/src/designer-components/checkbox/settingsForm.ts (3)
shesha-reactjs/src/designer-components/attachmentsEditor/settings.ts (1)
getSettings(9-962)shesha-reactjs/src/designer-components/fileUpload/settingsForm.ts (1)
getSettings(10-892)shesha-reactjs/src/providers/form/models.ts (1)
FormMarkupWithSettings(390-393)
shesha-reactjs/src/designer-components/checkbox/checkbox.tsx (4)
shesha-reactjs/src/hooks/formComponentHooks.ts (1)
useFormComponentStyles(179-254)shesha-reactjs/src/designer-components/checkbox/styles.ts (1)
useStyles(4-60)shesha-reactjs/src/designer-components/_common-migrations/migrateStyles.ts (1)
migratePrevStyles(100-110)shesha-reactjs/src/designer-components/checkbox/utils.ts (2)
defaultStyles(16-55)boxDefaultStyles(3-14)
shesha-reactjs/src/designer-components/propertiesTabs/style.ts (3)
shesha-reactjs/src/components/formDesigner/components/styles.ts (1)
useStyles(3-37)shesha-reactjs/src/designer-components/contextPropertyAutocomplete/styles.ts (1)
useStyles(3-36)shesha-reactjs/src/components/fileUpload/styles/styles.ts (1)
useStyles(3-343)
⏰ 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). (1)
- GitHub Check: build-attempt
| const { namePrefix, wrapperCol: formItemWrapperCol, labelCol: formItemlabelCol } = formItem; | ||
|
|
||
| const layout = useMemo(() => { | ||
| // Make sure the `wrapperCol` and `labelCol` from `FormItemProver` override the ones from the main form | ||
| return { labelCol: formItemlabelCol || labelCol, wrapperCol: formItemWrapperCol || wrapperCol }; | ||
| }, [formItemlabelCol, formItemWrapperCol]); | ||
| const settings = shaForm.settings; | ||
| const { styles } = useStyles(layout); | ||
|
|
There was a problem hiding this comment.
Wrong argument to useStyles; also variable naming collision.
You’re passing a { labelCol, wrapperCol } object into useStyles, which expects a layout string. Rename the local “layout” to “colLayout” and feed settings?.layout to useStyles.
- const layout = useMemo(() => {
+ const colLayout = useMemo(() => {
// Make sure the `wrapperCol` and `labelCol` from `FormItemProver` override the ones from the main form
return { labelCol: formItemlabelCol || labelCol, wrapperCol: formItemWrapperCol || wrapperCol };
- }, [formItemlabelCol, formItemWrapperCol]);
- const settings = shaForm.settings;
- const { styles } = useStyles(layout);
+ }, [formItemlabelCol, formItemWrapperCol, labelCol, wrapperCol]);
+ const settings = shaForm.settings;
+ const { styles } = useStyles(settings?.layout);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { namePrefix, wrapperCol: formItemWrapperCol, labelCol: formItemlabelCol } = formItem; | |
| const layout = useMemo(() => { | |
| // Make sure the `wrapperCol` and `labelCol` from `FormItemProver` override the ones from the main form | |
| return { labelCol: formItemlabelCol || labelCol, wrapperCol: formItemWrapperCol || wrapperCol }; | |
| }, [formItemlabelCol, formItemWrapperCol]); | |
| const settings = shaForm.settings; | |
| const { styles } = useStyles(layout); | |
| const { namePrefix, wrapperCol: formItemWrapperCol, labelCol: formItemlabelCol } = formItem; | |
| const colLayout = useMemo(() => { | |
| // Make sure the `wrapperCol` and `labelCol` from `FormItemProver` override the ones from the main form | |
| return { labelCol: formItemlabelCol || labelCol, wrapperCol: formItemWrapperCol || wrapperCol }; | |
| }, [formItemlabelCol, formItemWrapperCol, labelCol, wrapperCol]); | |
| const settings = shaForm.settings; | |
| const { styles } = useStyles(settings?.layout); |
shesha-reactjs/src/components/formDesigner/components/styles.ts
Outdated
Show resolved
Hide resolved
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx
Outdated
Show resolved
Hide resolved
| .addCollapsiblePanel({ | ||
| id: nanoid(), | ||
| propertyName: 'containerCustomStylePanel', | ||
| label: 'Custom Styles', | ||
| labelAlign: 'right', | ||
| ghost: true, | ||
| parentId: styleRouterId, | ||
| collapsible: 'header', | ||
| content: { | ||
| id: 'containerCustomStylePanel', | ||
| components: [...new DesignerToolbarSettings() | ||
| .addSettingsInput({ | ||
| id: nanoid(), | ||
| inputType: 'codeEditor', | ||
| propertyName: 'thumbnail.style', | ||
| hideLabel: false, | ||
| label: 'Style', | ||
| description: 'A script that returns the style of the element as an object. This should conform to CSSProperties', | ||
| parentId: 'containerCustomStylePanel', | ||
| }) | ||
| .toJson(), | ||
| ], | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Duplicate “Custom Styles” for the same property (thumbnail.style)
You have two panels labeled “Custom Styles” both editing thumbnail.style. This duplicates UI and risks conflicting edits. Keep just one.
Suggested removal (drop the earlier block and keep the later one):
- .addCollapsiblePanel({
- id: nanoid(),
- propertyName: 'containerCustomStylePanel',
- label: 'Custom Styles',
- labelAlign: 'right',
- ghost: true,
- parentId: styleRouterId,
- collapsible: 'header',
- content: {
- id: 'containerCustomStylePanel',
- components: [...new DesignerToolbarSettings()
- .addSettingsInput({
- id: nanoid(),
- inputType: 'codeEditor',
- propertyName: 'thumbnail.style',
- hideLabel: false,
- label: 'Style',
- description: 'A script that returns the style of the element as an object. This should conform to CSSProperties',
- parentId: 'containerCustomStylePanel',
- })
- .toJson(),
- ],
- },
- })Also applies to: 831-854
🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/fileUpload/settingsForm.ts around
lines 463 to 486, there is a duplicated "Custom Styles" collapsible panel that
edits the same property thumbnail.style; remove this earlier block so only one
panel remains to avoid conflicting edits (the same duplicate also exists around
lines 831 to 854—remove the duplicate there instead or keep only the intended
single panel), ensuring any references like parentId or propertyName are not
left dangling after removal.
| * { | ||
| font-size: 12px; | ||
| --ant-control-height: 0 !important; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Universal selector with --ant-control-height: 0 may break form controls.
The universal selector * with !important is overly aggressive and could cause critical issues:
-
--ant-control-height: 0 !important: This CSS variable controls the height of Ant Design form controls (inputs, selects, buttons). Setting it to0could render these controls invisible or unusable. -
font-size: 12px: Applying to all descendants may cause unintended effects on nested components, icons, or third-party widgets that expect different font sizing. -
Maintainability: Universal selectors with
!importantcreate specificity wars and are difficult to debug.
Consider using more specific selectors that target only the elements you need to style. For example:
- * {
- font-size: 12px;
- --ant-control-height: 0 !important;
- }
+ .ant-tabs-tab,
+ .ant-form-item-label,
+ .ant-form-item-control {
+ font-size: 12px;
+ }Please verify that form controls (Input, Select, Button, DatePicker, etc.) render correctly within the properties tabs after this change. Run the following verification:
🏁 Script executed:
#!/bin/bash
# Description: Search for Ant Design form controls used within properties tabs that might be affected
# Find components that render form controls within designer properties
rg -n --type=tsx --type=ts -C3 'Input|Select|Button|DatePicker|Checkbox|Radio' shesha-reactjs/src/designer-components/propertiesTabs/Length of output: 27
🏁 Script executed:
#!/bin/bash
# Find Ant Design form controls in propertiesTabs with corrected ripgrep syntax
rg -n 'Input|Select|Button|DatePicker|Checkbox|Radio' shesha-reactjs/src/designer-components/propertiesTabs/ -C2
# Also locate the style.ts file and show its full content
echo "=== Content of style.ts ==="
fd 'style\.ts' shesha-reactjs/src/designer-components/propertiesTabs/ -exec cat {} \;
# Check if there are any .tsx or .ts files in the directory
echo "=== Files in propertiesTabs ==="
fd . shesha-reactjs/src/designer-components/propertiesTabs/ -type fLength of output: 15883
🏁 Script executed:
#!/bin/bash
# Locate and display the style.ts file correctly
find shesha-reactjs/src/designer-components/propertiesTabs -name "style.ts" -type f
# Display content using cat directly
find shesha-reactjs/src/designer-components/propertiesTabs -name "style.ts" -type f | xargs catLength of output: 997
Critical: Universal selector with --ant-control-height: 0 breaks form controls in properties tabs.
The code at lines 23-26 uses a universal selector * within the .content scoped styles to set --ant-control-height: 0 !important. This affects all descendant elements and will render form controls (Input, Select, Checkbox, Button) unusable by collapsing their heights to 0.
Evidence: Input, Select, and Checkbox components from Ant Design are actively rendered in settings.tsx and searchableTabsComponent.tsx within this component. The --ant-control-height CSS variable controls the computed height of these controls—setting it to 0 breaks them.
Replace the universal selector with targeted selectors:
- * {
- font-size: 12px;
- --ant-control-height: 0 !important;
- }
+ .ant-form-item {
+ font-size: 12px;
+ }If you need to target form field labels or text specifically, use more precise selectors like .ant-form-item-label or .ant-form-item-label > label instead of affecting all descendants.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * { | |
| font-size: 12px; | |
| --ant-control-height: 0 !important; | |
| } | |
| .ant-form-item { | |
| font-size: 12px; | |
| } |
🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/propertiesTabs/style.ts around lines
23 to 26, the universal selector (*) sets --ant-control-height: 0 !important
which collapses Ant Design form controls; remove the global * selector and
instead apply the intended styling to precise selectors (e.g.
.ant-form-item-label or .ant-form-item-label > label) so only label/text styles
are affected, and ensure you do not override --ant-control-height for inputs,
selects, checkboxes, or buttons (remove the !important and the variable
assignment from any global rule).
shesha-reactjs/src/designer-components/settingsInput/settingsInput.tsx
Outdated
Show resolved
Hide resolved
shesha-reactjs/src/designer-components/styleBox/components/input.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
shesha-reactjs/src/components/reactTable/index.tsx (1)
448-448: Add optional chaining to prevent null reference error.
shaForm.formModeis accessed without optional chaining, butuseShaFormInstanceOrUndefined()(line 120) may returnnullorundefined. This will throw a runtime error when the form instance is not available.Apply this diff to add optional chaining:
- const zoomLevel = shaForm.formMode === "designer" ? canvasState?.zoom ?? 100 : 100; + const zoomLevel = shaForm?.formMode === "designer" ? canvasState?.zoom ?? 100 : 100;
🧹 Nitpick comments (3)
shesha-reactjs/src/designer-components/dateField/datePickerWrapper.tsx (1)
222-222: Remove redundant width specification.Since
finalStylesalready includeswidth: '100%'(Line 43), explicitly setting it again is redundant. For consistency with theRangePicker(Line 197) andReadOnlyDisplayFormItem(Line 207), usestyle={finalStyles}directly.Apply this diff:
- style={{ ...finalStyles, width: '100%' }} + style={finalStyles}shesha-reactjs/src/designer-components/attachmentsEditor/attachmentsEditor.tsx (2)
162-162: Don’t overwrite existing handler; prefer nullish coalescing and reuse sanitized mapping.Use
??and keep any preexistingonChangeCustom. Also coalesce boolean explicitly.- .add<IAttachmentsEditorProps>(8, (prev) => ({ ...prev, downloadZip: prev.downloadZip || false, propertyName: prev.propertyName ?? '', onChangeCustom: prev.onFileChanged })) + .add<IAttachmentsEditorProps>(8, (prev) => ({ + ...prev, + downloadZip: prev.downloadZip ?? false, + propertyName: prev.propertyName ?? '', + // preserve if already set; otherwise map from the sanitized legacy handler + onChangeCustom: prev.onChangeCustom ?? migrateFormApi.withoutFormData?.(prev?.onFileChanged) ?? prev?.onFileChanged + }))Please confirm this matches the intended migration semantics.
182-188: Confirm container-vs-root precedence in merge order.
{ ...root, ...container }lets container override root on conflicts. If the opposite is desired, swap the spread order.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
shesha-reactjs/src/components/reactTable/index.tsx(1 hunks)shesha-reactjs/src/designer-components/_settings/styles/styles.ts(1 hunks)shesha-reactjs/src/designer-components/attachmentsEditor/attachmentsEditor.tsx(1 hunks)shesha-reactjs/src/designer-components/attachmentsEditor/settings.ts(12 hunks)shesha-reactjs/src/designer-components/dateField/datePickerWrapper.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- shesha-reactjs/src/designer-components/_settings/styles/styles.ts
- shesha-reactjs/src/designer-components/attachmentsEditor/settings.ts
⏰ 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). (1)
- GitHub Check: build-attempt
🔇 Additional comments (1)
shesha-reactjs/src/designer-components/dateField/datePickerWrapper.tsx (1)
43-43: Enforcing full width is appropriate for the PR objective.The change ensures all date picker variants render at full width by overriding any width from
additionalStyles. This aligns with the PR's goal to fix width property issues.
| const migrateStyleLevel = (prevStyles: any): any => { | ||
| if (!prevStyles) return prevStyles; | ||
|
|
||
| const containerStyles = prevStyles.container || {}; | ||
| const rootStyles = { ...prevStyles }; | ||
| delete rootStyles.container; | ||
|
|
||
| return { | ||
| ...rootStyles, | ||
| ...containerStyles, | ||
| thumbnail: rootStyles, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Fix potential thumbnail nesting and make the style migration idempotent.
Current logic can create thumbnail.thumbnail if prevStyles already had thumbnail. Also safer to clone and early‑return when already migrated.
- const migrateStyleLevel = (prevStyles: any): any => {
- if (!prevStyles) return prevStyles;
-
- const containerStyles = prevStyles.container || {};
- const rootStyles = { ...prevStyles };
- delete rootStyles.container;
-
- return {
- ...rootStyles,
- ...containerStyles,
- thumbnail: rootStyles,
- };
- };
+ const migrateStyleLevel = (prevStyles: Record<string, any> | undefined): Record<string, any> | undefined => {
+ if (!prevStyles) return prevStyles;
+ // Already migrated: has thumbnail and no container — don’t reprocess
+ if (!('container' in prevStyles) && ('thumbnail' in prevStyles)) return prevStyles;
+
+ const {
+ container: containerStyles = {},
+ thumbnail: _ignoreExistingThumbnail,
+ ...rootWithoutContainerAndThumbnail
+ } = prevStyles;
+
+ return {
+ ...rootWithoutContainerAndThumbnail,
+ ...containerStyles,
+ // clone to avoid accidental aliasing and remove any preexisting nested thumbnail
+ thumbnail: { ...rootWithoutContainerAndThumbnail },
+ };
+ };A quick grep to ensure no code expects thumbnail.thumbnail:
---
🏁 Script executed:
```shell
#!/bin/bash
# Fetch the actual file content around lines 168-180 to confirm the code state
fd -e tsx -e ts 'attachmentsEditor' | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && sed -n "160,190p" "{}"'
Length of output: 1408
🏁 Script executed:
#!/bin/bash
# Search for usages of thumbnail properties to understand expected structure
rg -nP -C3 'thumbnail\s*[:\.]' --type ts --type tsx | head -40Length of output: 27
🏁 Script executed:
#!/bin/bash
# Find where migrateStyleLevel is called and in what context
rg -nP 'migrateStyleLevel' --type ts --type tsx -C2Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for tests related to attachments editor styling or migration
fd -e spec.ts -e test.ts -e test.tsx | xargs rg -l 'attachments|migrateStyle' 2>/dev/nullLength of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for thumbnail property access without type constraint
rg -nP 'thumbnail' attachmentsEditor.tsx -C2
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Find where styles are consumed; search for desktop, mobile, tablet access patterns
rg -nP '(desktop|mobile|tablet)\s*[:\.]' attachmentsEditor.tsx -C2 | head -50Length of output: 61
🏁 Script executed:
#!/bin/bash
# Check the migration step registration to see if step 9 runs conditionally
cat shesha-reactjs/src/designer-components/attachmentsEditor/attachmentsEditor.tsx | grep -A 50 "\.add<IAttachmentsEditorProps>(9"Length of output: 822
🏁 Script executed:
#!/bin/bash
# Search across the codebase for thumbnail property access to understand expected structure
rg -n 'thumbnail\s*\[\s*["\x27]|\bthumbnail\s*\.\s*\w+' --glob='*.ts' --glob='*.js' | head -30Length of output: 4709
🏁 Script executed:
#!/bin/bash
# Look for style processors or renderers that might consume the restructured styles
rg -nP 'def.*Styles|processStyles|applyStyles|renderStyle' --glob='*.ts' --glob='*.js' | head -20Length of output: 2208
Ensure migration is idempotent and prevent thumbnail nesting.
The migrateStyleLevel function lacks a guard against re-execution. If step 9 runs twice (or thumbnail exists in prevStyles), it creates nested structures. The fix adds an idempotency check, excludes pre-existing thumbnail, and improves typing:
const migrateStyleLevel = (prevStyles: any): any => {
if (!prevStyles) return prevStyles;
const containerStyles = prevStyles.container || {};
const rootStyles = { ...prevStyles };
delete rootStyles.container;
return {
...rootStyles,
...containerStyles,
thumbnail: rootStyles,
};
};Should be:
const migrateStyleLevel = (prevStyles: Record<string, any> | undefined): Record<string, any> | undefined => {
if (!prevStyles) return prevStyles;
// Already migrated: no container and has thumbnail — skip
if (!('container' in prevStyles) && ('thumbnail' in prevStyles)) return prevStyles;
const {
container: containerStyles = {},
thumbnail: _ignoreExistingThumbnail,
...rootWithoutContainerAndThumbnail
} = prevStyles;
return {
...rootWithoutContainerAndThumbnail,
...containerStyles,
thumbnail: { ...rootWithoutContainerAndThumbnail },
};
};🤖 Prompt for AI Agents
In
shesha-reactjs/src/designer-components/attachmentsEditor/attachmentsEditor.tsx
around lines 168 to 180, migrateStyleLevel is not idempotent and will nest
thumbnail objects if run twice or if prevStyles already contains thumbnail;
update the function to return early when prevStyles.thumbnail exists (or when
thumbnail key present), ensure it copies and merges container into the root
without overwriting an existing thumbnail, and explicitly exclude any existing
thumbnail from being used as the root (i.e., delete or omit prevStyles.thumbnail
before constructing rootStyles); also tighten typings from any to a proper
interface so the function signature reflects the expected shape and the return
type is accurate.
- Removed unused form item margin prop in ConfigurableFormRenderer. - Updated style handling in FileUpload component for better dragger support. - Enhanced ConfigurableFormItemLive to apply default margins and improved layout calculations. - Refactored dimension handling in dimensionUtils and stylingUtils for better unit management. - Fixed minor style issues in various components, including Checkbox and SettingsInput. - Added new utility functions for dimension parsing and calculation.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
shesha-reactjs/src/components/fileUpload/index.tsx (1)
35-36: RemovethumbnailWidthandthumbnailHeightfrom interface definitions—they're unused legacy properties.Verification confirms these props are declared in interfaces but never destructured or referenced in any component implementation. They appear only in type definitions across four files (fileUpload, customFile, storedFilesRendererBase, attachmentsEditor) with zero actual usage. Since the component already uses the new
style?: CSSPropertiesapproach for styling, these legacy properties should be removed to prevent confusion and maintain a clean API.shesha-reactjs/src/designer-components/styleBox/components/utils.ts (1)
13-16: Zero-stripping breaks valid values like '0px' and '0'.The pattern
/\b0+/gremoves the leading zero in '0px' → 'px' and can also turn '0' into ''.Apply this safer prefix-only replacement:
- [`${type}${capitalizeFirstLetter(direction)}`]: value.replace(/\b0+/g, ''), + [`${type}${capitalizeFirstLetter(direction)}`]: value.replace(/^0+(?=\d)/, ''),This trims only leading zeros immediately before a digit, preserving '0', '0px', and '0.5'.
♻️ Duplicate comments (3)
shesha-reactjs/src/designer-components/styleBox/components/utils.ts (1)
19-22: Resolved: return type is consistently string now.Returning the string literal '0' fixes the earlier mismatch. LGTM.
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx (1)
124-136: Good: exhaustive dependencies added for renderComponentModel.Including activeDevice and dimensionsStyles prevents stale memoization.
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx (1)
31-34: Potential crash: settings may be undefined here.shaForm.settings can be undefined; accessing settings.layout would throw.
- const settings = shaForm.settings; - const { styles } = useStyles(settings.layout); + const settings = shaForm.settings; + const { styles } = useStyles(settings?.layout);
🧹 Nitpick comments (5)
shesha-reactjs/src/designer-components/checkbox/checkbox.tsx (1)
87-92: Use nullish coalescing operator for clearer intent and safer fallback handling.Migration step 6 restructures the data model by moving dimensions from device level to the
checkboxproperty. This change is intentional and safe—no code reads from device-leveldimensionsafter migration.However, consider refactoring the fallback expressions from
||to??for semantic clarity. The logical OR operator treats empty objects as falsy, while nullish coalescing only treatsnullandundefinedas falsy, better expressing the intent that you're protecting against missing device properties, not falsy values.Suggested refactor:
.add<ICheckboxComponentProps>(6, (prev) => ( { ...prev, - desktop: { ...prev.desktop, dimensions: {}, checkbox: boxDefaultStyles(prev.desktop || prev) }, - mobile: { ...prev.mobile, dimensions: {}, checkbox: boxDefaultStyles(prev.mobile || prev) }, - tablet: { ...prev.tablet, dimensions: {}, checkbox: boxDefaultStyles(prev.tablet || prev) }, + desktop: { ...prev.desktop, dimensions: {}, checkbox: boxDefaultStyles(prev.desktop ?? prev) }, + mobile: { ...prev.mobile, dimensions: {}, checkbox: boxDefaultStyles(prev.mobile ?? prev) }, + tablet: { ...prev.tablet, dimensions: {}, checkbox: boxDefaultStyles(prev.tablet ?? prev) }, }))shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (1)
106-112: Normalize margins to CSS strings for consistency.createRootContainerStyle uses addPx; createFormItemStyle should do the same to avoid unit drift between contexts.
- ...(formMode !== 'designer' && { - marginLeft, - marginRight, - marginBottom, - marginTop, - }), + ...(formMode !== 'designer' && { + marginLeft: addPx(marginLeft), + marginRight: addPx(marginRight), + marginBottom: addPx(marginBottom), + marginTop: addPx(marginTop), + }),Remember to import addPx at the top:
-import React, { CSSProperties } from 'react'; +import React, { CSSProperties } from 'react'; +import { addPx } from '@/utils/style';shesha-reactjs/src/utils/style.ts (1)
41-50: Preserve CSS function values in addPx (calc/var/min/max/clamp).Currently, addPx returns undefined for these, silently dropping valid styles.
export const addPx = (value: number | string | null | undefined): string | undefined => { - const parsed = parseDimension(value); + if (typeof value === 'string') { + const v = value.trim(); + if (/^(calc|var|min|max|clamp)\(/i.test(v)) return v; + } + const parsed = parseDimension(value); if (!parsed) return undefined; if (parsed.unit === 'auto' || parsed.unit === 'none') { return parsed.unit; } return `${parsed.value}${parsed.unit}`; };shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx (1)
21-25: Avoid calling useShaFormInstance twice; reuse the same instance.Minor cleanup to prevent redundant hook calls.
- const { getPublicFormApi } = useShaFormInstance(); + const shaForm = useShaFormInstance(); + const { getPublicFormApi } = shaForm; const getFormData = getPublicFormApi().getFormData; - const formItem = useFormItem(); - const shaForm = useShaFormInstance(); + const formItem = useFormItem();shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx (1)
119-146: Remove JSON stringify/parse for styles; pass objects directly and fix deps.Stringifying margins/padding is unnecessary and obscures dependencies.
- const paddingStyles = JSON.stringify(stylingBoxPadding); - const marginStyles = JSON.stringify(stylingBoxMargin); + // use objects directly; no serialization needed const componentDimensions = getComponentDimensions(typeInfo, dimensionsStyles); const renderComponentModel = useMemo(() => { const deviceDimensions = getDeviceDimensions(typeInfo, dimensionsStyles); return { ...componentModel, [activeDevice]: { ...desktopConfig, dimensions: deviceDimensions, }, - // ...(formMode === 'designer' ? {stylingBox: paddingStyles} : {stylingBox: JSON.stringify({...stylingBoxPadding, ...stylingBoxMargin})}), flexBasis: getDeviceFlexBasis(dimensionsStyles), }; - }, [componentModel, desktopConfig, paddingStyles, originalDimensions, formMode, typeInfo, activeDevice, dimensionsStyles]); + }, [componentModel, desktopConfig, originalDimensions, formMode, typeInfo, activeDevice, dimensionsStyles]); - const rootContainerStyle = useMemo(() => { - return createRootContainerStyle( - componentDimensions, - { ...JSON.parse(marginStyles) }, - originalDimensions, - typeInfo.isInput, - ); - }, [componentDimensions, marginTop, marginBottom, marginLeft, marginRight, originalDimensions, hasLabel]); + const rootContainerStyle = useMemo(() => { + return createRootContainerStyle( + componentDimensions, + stylingBoxMargin, + originalDimensions, + typeInfo.isInput, + ); + }, [componentDimensions, stylingBoxMargin, originalDimensions, typeInfo.isInput]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
shesha-reactjs/src/components/fileUpload/index.tsx(5 hunks)shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx(3 hunks)shesha-reactjs/src/components/formDesigner/components/styles.ts(1 hunks)shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx(7 hunks)shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts(1 hunks)shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts(1 hunks)shesha-reactjs/src/components/mainLayout/styles/indexStyles.ts(1 hunks)shesha-reactjs/src/components/reactTable/index.tsx(1 hunks)shesha-reactjs/src/designer-components/checkbox/checkbox.tsx(4 hunks)shesha-reactjs/src/designer-components/checkbox/styles.ts(2 hunks)shesha-reactjs/src/designer-components/collapsiblePanel/utils.ts(2 hunks)shesha-reactjs/src/designer-components/propertiesTabs/style.ts(2 hunks)shesha-reactjs/src/designer-components/settingsInput/settingsInput.tsx(1 hunks)shesha-reactjs/src/designer-components/styleBox/components/input.tsx(2 hunks)shesha-reactjs/src/designer-components/styleBox/components/utils.ts(1 hunks)shesha-reactjs/src/utils/style.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- shesha-reactjs/src/designer-components/checkbox/styles.ts
- shesha-reactjs/src/designer-components/styleBox/components/input.tsx
- shesha-reactjs/src/designer-components/collapsiblePanel/utils.ts
- shesha-reactjs/src/components/mainLayout/styles/indexStyles.ts
- shesha-reactjs/src/components/reactTable/index.tsx
- shesha-reactjs/src/components/formDesigner/components/styles.ts
- shesha-reactjs/src/designer-components/propertiesTabs/style.ts
- shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
PR: shesha-io/shesha-framework#3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.
Applied to files:
shesha-reactjs/src/designer-components/checkbox/checkbox.tsx
🧬 Code graph analysis (6)
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx (3)
shesha-reactjs/src/providers/form/store/shaFormInstance.tsx (1)
settings(241-243)shesha-reactjs/src/components/formDesigner/components/styles.ts (1)
useStyles(3-37)shesha-reactjs/src/utils/style.ts (1)
addPx(41-50)
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (2)
shesha-reactjs/src/utils/style.ts (3)
canAddToCalc(57-69)parseDimension(13-34)addPx(41-50)shesha-reactjs/src/components/formDesigner/utils/componentTypeUtils.ts (1)
ComponentTypeInfo(3-6)
shesha-reactjs/src/utils/style.ts (1)
shesha-reactjs/src/utils/nullables.ts (1)
isDefined(2-4)
shesha-reactjs/src/designer-components/styleBox/components/utils.ts (2)
shesha-reactjs/src/utils/object.ts (1)
jsonSafeParse(10-20)shesha-reactjs/src/utils/string.ts (1)
capitalizeFirstLetter(139-143)
shesha-reactjs/src/designer-components/checkbox/checkbox.tsx (5)
shesha-reactjs/src/hooks/formComponentHooks.ts (1)
useFormComponentStyles(179-254)shesha-reactjs/src/designer-components/checkbox/styles.ts (1)
useStyles(4-60)shesha-reactjs/src/designer-components/checkbox/settingsForm.ts (1)
getSettings(9-715)shesha-reactjs/src/designer-components/_common-migrations/migrateStyles.ts (1)
migratePrevStyles(100-110)shesha-reactjs/src/designer-components/checkbox/utils.ts (2)
defaultStyles(16-55)boxDefaultStyles(3-14)
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx (6)
shesha-reactjs/src/providers/form/store/shaFormInstance.tsx (2)
formSettings(150-152)formMode(154-156)shesha-reactjs/src/providers/canvas/index.tsx (1)
useCanvas(146-146)shesha-reactjs/src/components/formDesigner/utils/componentTypeUtils.ts (1)
getComponentTypeInfo(8-16)shesha-reactjs/src/hooks/formComponentHooks.ts (1)
useFormComponentStyles(179-254)shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (3)
getComponentDimensions(15-47)getDeviceDimensions(49-59)getDeviceFlexBasis(61-65)shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (1)
createRootContainerStyle(48-89)
⏰ 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). (1)
- GitHub Check: build-attempt
🔇 Additional comments (6)
shesha-reactjs/src/designer-components/settingsInput/settingsInput.tsx (1)
35-35: Good fix for explicit units; verify button width is intentional.The addition of the 'px' unit addresses the previous review comment and ensures consistent rendering across browsers. However, 24px is quite small for a button (typical minimum is 32-40px). Please verify this is the intended width for the button type.
shesha-reactjs/src/components/fileUpload/index.tsx (2)
54-73: LGTM!The
styleprop is correctly destructured and passed through to theuseStyleshook. The props object structure properly combines the style with the model configuration.
224-224: LGTM! Previous issue resolved.The conditional style assignment now correctly uses a ternary operator, returning either the
styleobject orundefined—never a boolean. This resolves the typing/runtime issue flagged in the previous review.shesha-reactjs/src/designer-components/checkbox/checkbox.tsx (3)
20-24: LGTM!The new imports are properly structured and all are utilized within the file.
52-52: LGTM!The style merging strategy correctly overlays box-specific dimensions on top of the final style, allowing independent control of the checkbox box appearance.
71-72: LGTM!The signature changes are consistent and align with the refactored
getSettings()function that now returns static form markup instead of requiring runtime data.
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx
Outdated
Show resolved
Hide resolved
| dataTypeSupported: ({ dataType }) => dataType === DataTypes.boolean, | ||
| calculateModel: (model, allData) => ({ eventHandlers: getAllEventHandlers(model, allData) }), | ||
| Factory: ({ model, calculatedModel }) => { | ||
| const boxDimensions = useFormComponentStyles({...model.checkbox}).dimensionsStyles; |
There was a problem hiding this comment.
Add defensive check for undefined model.checkbox.
Components that haven't been migrated to step 6 won't have model.checkbox defined. While {...undefined} evaluates to {}, passing an empty object to useFormComponentStyles may not return valid dimensionsStyles, potentially causing rendering issues.
Apply this diff to add a fallback:
- const boxDimensions = useFormComponentStyles({...model.checkbox}).dimensionsStyles;
+ const boxDimensions = model.checkbox
+ ? useFormComponentStyles({...model.checkbox}).dimensionsStyles
+ : {};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const boxDimensions = useFormComponentStyles({...model.checkbox}).dimensionsStyles; | |
| const boxDimensions = model.checkbox | |
| ? useFormComponentStyles({...model.checkbox}).dimensionsStyles | |
| : {}; |
🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/checkbox/checkbox.tsx around line 46,
add a defensive fallback for undefined model.checkbox before calling
useFormComponentStyles so you don't pass an unexpected empty/invalid shape;
check if model.checkbox is defined and pass model.checkbox or an explicit
empty/default object (e.g., {}) into useFormComponentStyles, then ensure
dimensionsStyles falls back to a safe default value if the hook returns
undefined (so the component can render safely for pre-step-6 models).
…com/czwe-01/shesha-framework into thulasizwe/en/form-components-width
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (1)
79-84: Avoid setting height to an empty string for inputs.An empty string is a brittle sentinel; omit the property instead by using
undefined.Apply this diff to fix:
height: isDataTableContext ? '100%' : isInput - ? '' + ? undefined : dimensionsStyles?.height,
🧹 Nitpick comments (1)
shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (1)
61-65: Consider inlining this trivial function.This single-line accessor could be inlined at call sites for simplicity, though keeping it as a utility is acceptable for consistency with other dimension functions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx(4 hunks)shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts(1 hunks)shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts(1 hunks)shesha-reactjs/src/designer-components/checkbox/checkbox.tsx(4 hunks)shesha-reactjs/src/designer-components/checkbox/interfaces.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- shesha-reactjs/src/designer-components/checkbox/interfaces.ts
- shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
PR: shesha-io/shesha-framework#3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.
Applied to files:
shesha-reactjs/src/designer-components/checkbox/checkbox.tsx
🧬 Code graph analysis (3)
shesha-reactjs/src/designer-components/checkbox/checkbox.tsx (5)
shesha-reactjs/src/hooks/formComponentHooks.ts (1)
useFormComponentStyles(179-254)shesha-reactjs/src/designer-components/checkbox/settingsForm.ts (1)
getSettings(9-715)shesha-reactjs/src/designer-components/checkbox/interfaces.ts (1)
ICheckboxComponentProps(3-5)shesha-reactjs/src/designer-components/_common-migrations/migrateStyles.ts (1)
migratePrevStyles(100-110)shesha-reactjs/src/designer-components/checkbox/utils.ts (2)
defaultStyles(16-55)boxDefaultStyles(3-14)
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (2)
shesha-reactjs/src/utils/style.ts (2)
addPx(41-50)hasNumber(74-74)shesha-reactjs/src/components/formDesigner/utils/componentTypeUtils.ts (1)
ComponentTypeInfo(3-6)
shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (1)
shesha-reactjs/src/components/formDesigner/utils/componentTypeUtils.ts (1)
ComponentTypeInfo(3-6)
⏰ 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). (1)
- GitHub Check: build-attempt
🔇 Additional comments (10)
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (2)
5-14: LGTM!The
StyleConfiginterface is well-defined with appropriate types for CSS margin and padding properties.
43-48: The original review comment is incorrect and should be disregarded.The concern assumes
undefinedcan reachcalc()expressions, but this scenario cannot occur:
- Margin values always have fallback: Lines 27-30 use
stylingBox?.marginX || 0, ensuringaddPxreceives0(notundefined)addPx(0)returns a valid string:parseDimension(0)returns{ value: 0, unit: 'px' }(line 16-17), which is truthy, soaddPx(0)returns"0px"(line 49)calc()expressions are valid: The result is valid CSS likecalc(100px + 0px + 0px)The code is safe as written. No changes are needed.
Likely an incorrect or invalid review comment.
shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (3)
5-13: LGTM!The
DimensionConfiginterface clearly defines dimension-related CSS properties.
15-47: Good implementation of dimension logic.The function correctly handles different contexts (data table, input) and uses
'auto'for input height, which is preferable to an empty string.
49-59: LGTM!The device-specific dimension logic is straightforward and appropriate for responsive layouts.
shesha-reactjs/src/designer-components/checkbox/checkbox.tsx (5)
20-22: LGTM!The new imports are properly used throughout the component for the styling functionality.
47-52: LGTM!The style merging correctly combines
finalStylewithboxDimensions, with box dimensions taking precedence as intended.
71-72: LGTM!The updated signatures correctly align with the new
getSettings()function that takes no parameters, as confirmed in the relevant code snippets.
87-92: Critical: Property name mismatch between migrator and code.The migrator sets
checkboxproperty on each breakpoint, but line 46 accessesmodel.box. This mismatch means the styling won't work after migration.Apply this diff to fix the property name:
.add<ICheckboxComponentProps>(6, (prev) => ( { ...prev, - desktop: { ...prev.desktop, dimensions: {}, checkbox: boxDefaultStyles(prev.desktop || prev) }, - mobile: { ...prev.mobile, dimensions: {}, checkbox: boxDefaultStyles(prev.mobile || prev) }, - tablet: { ...prev.tablet, dimensions: {}, checkbox: boxDefaultStyles(prev.tablet || prev) }, + desktop: { ...prev.desktop, dimensions: {}, box: boxDefaultStyles(prev.desktop || prev) }, + mobile: { ...prev.mobile, dimensions: {}, box: boxDefaultStyles(prev.mobile || prev) }, + tablet: { ...prev.tablet, dimensions: {}, box: boxDefaultStyles(prev.tablet || prev) }, }))Likely an incorrect or invalid review comment.
89-91: Clarify: Step 6 migration loses previous dimension values during restructuring.The
dimensions: {}clearing is intentional, restructuring dimensions into theboxproperty. However,boxDefaultStyles()only preserves the previous height value—width is reset to hardcoded'16px', and min/max properties revert to defaults (lines 6-11 of utils.ts). This causes loss of any previously set width or dimension constraints for existing checkbox instances, potentially causing layout regressions.Verify whether this data loss is acceptable or if dimension values should be fully preserved during the migration.
| height: isDataTableContext | ||
| ? '100%' | ||
| : isInput | ||
| ? '' | ||
| : dimensionsStyles?.height, | ||
| }; |
There was a problem hiding this comment.
Inconsistent handling of input height across utilities.
This file uses '' (empty string) for input height, while dimensionUtils.ts line 28 uses 'auto'. Consider aligning both utilities to use the same approach—either 'auto' or undefined—for consistency.
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts around lines
79 to 84, the input height is set to an empty string (''), which is inconsistent
with dimensionUtils.ts line 28 that uses 'auto'; change the input branch to
return the same value as dimensionUtils (either 'auto' or undefined) across both
utilities—pick 'auto' if you want explicit CSS height or undefined to let the
browser default, update this file to use the chosen value and adjust
dimensionUtils.ts if necessary so both utilities use the identical approach.
| dataTypeSupported: ({ dataType }) => dataType === DataTypes.boolean, | ||
| calculateModel: (model, allData) => ({ eventHandlers: getAllEventHandlers(model, allData) }), | ||
| Factory: ({ model, calculatedModel }) => { | ||
| const boxDimensions = useFormComponentStyles({...model.box}).dimensionsStyles; |
There was a problem hiding this comment.
Add defensive check for undefined model.box.
Components not yet migrated to step 6 won't have model.box defined. While {...undefined} evaluates to {}, passing an empty object to useFormComponentStyles may not return valid dimensionsStyles, potentially causing layout issues.
Apply this diff to add a fallback:
- const boxDimensions = useFormComponentStyles({...model.box}).dimensionsStyles;
+ const boxDimensions = model.box
+ ? useFormComponentStyles({...model.box}).dimensionsStyles
+ : {};🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/checkbox/checkbox.tsx around line 46,
add a defensive fallback for missing model.box before calling
useFormComponentStyles: ensure you pass a safe object (e.g. model.box ?? {}) and
guard the result so boxDimensions is a valid object (e.g.
useFormComponentStyles(...).dimensionsStyles || {}) to avoid undefined layout
values for components not migrated to step 6.
- Introduced getDeviceDimensions utility for improved width and height calculations based on styling margins. - Updated ConfigurableFormItemLive to utilize device dimensions for responsive design in designer mode. - Refactored getComponentDimensions to streamline dimension calculations and removed unused input checks. - Adjusted styling logic in createRootContainerStyle to ensure consistent height management. - Minor updates to button and numberField components for better styling integration.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shesha-reactjs/src/designer-components/button/button.tsx (1)
30-39: Verify that later style spreads don't undermine the designer mode width override.The designer mode width is applied first in the spread sequence (line 31), but subsequent spreads of
stylingBoxAsCSSandjsStyle(lines 36-37) could contain width properties that override the intended100%width in designer mode. If the goal is to enforce100%width in designer mode regardless of other styles, consider moving the designer mode width override to the end of the spread sequence:const finalStyle = { - ...shaForm.formMode === 'designer' ? { width: '100%'}: model.allStyles.dimensionsStyles, + ...(shaForm.formMode !== 'designer' && model.allStyles.dimensionsStyles), ...(['primary', 'default'].includes(model.buttonType) && !model.readOnly && model.allStyles.borderStyles), ...model.allStyles.fontStyles, ...(['dashed', 'default'].includes(model.buttonType) && !model.readOnly && model.allStyles.backgroundStyles), ...(['primary', 'default'].includes(model.buttonType) && model.allStyles.shadowStyles), ...model.allStyles.stylingBoxAsCSS, ...model.allStyles.jsStyle, + ...(shaForm.formMode === 'designer' ? { width: '100%' } : {}), justifyContent: model.font?.align, };This ensures that the designer mode width takes precedence over all other width settings.
Minor: Spacing inconsistency (optional nitpick)
Line 31 has inconsistent spacing around the ternary operator ('100%'}: modelhas two spaces after the colon). Consider normalizing to single space for consistency.
♻️ Duplicate comments (1)
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx (1)
110-117: Fix reversed margin fallbacks; use nullish coalescing.Each line returns form defaults when a component value exists. Should return component value when set, else fall back. Also update deps.
- const stylingBoxMargin = useMemo(() => { - return { - marginTop: marginTop ? top : marginTop, - marginBottom: marginBottom ? bottom : marginBottom, - marginLeft: marginLeft ? left : marginLeft, - marginRight: marginRight ? right : marginRight, - }; - }, [formMode, originalStylingBox, desktopConfig.stylingBox]); + const stylingBoxMargin = useMemo(() => { + return { + marginTop: marginTop ?? top, + marginBottom: marginBottom ?? bottom, + marginLeft: marginLeft ?? left, + marginRight: marginRight ?? right, + }; + }, [marginTop, marginBottom, marginLeft, marginRight, top, bottom, left, right]);
🧹 Nitpick comments (3)
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx (1)
28-31: Complete useMemo dependency list.Include
labelColandwrapperColsince they’re read inside the memo.- }, [formItemlabelCol, formItemWrapperCol]); + }, [formItemlabelCol, formItemWrapperCol, labelCol, wrapperCol]);shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx (1)
138-145: Stabilize rootContainerStyle memo dependencies.Depend on
marginStyles(the actual input) instead of individual margin variables to avoid stale parses.- }, [componentDimensions, marginTop, marginBottom, marginLeft, marginRight, originalDimensions, hasLabel]); + }, [componentDimensions, marginStyles, originalDimensions, hasLabel]);shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (1)
5-13: DimensionConfig should permit numbers too.CSSProperties fields can be
number | string; widen the interface to avoid narrowing.export interface DimensionConfig { - width?: string; - height?: string; - maxWidth?: string; - minWidth?: string; - maxHeight?: string; - minHeight?: string; - flexBasis?: string; + width?: string | number; + height?: string | number; + maxWidth?: string | number; + minWidth?: string | number; + maxHeight?: string | number; + minHeight?: string | number; + flexBasis?: string | number; } @@ - const getDimensionValue = (dimensionType: keyof DimensionConfig): string | number => { + const getDimensionValue = (dimensionType: keyof DimensionConfig): string | number => { if (isDataTableContext) return '100%'; return dimensionsStyles?.[dimensionType]; };Also applies to: 29-44
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx(4 hunks)shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx(7 hunks)shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts(1 hunks)shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts(1 hunks)shesha-reactjs/src/designer-components/_settings/utils/dimensions/utils.tsx(1 hunks)shesha-reactjs/src/designer-components/button/button.tsx(2 hunks)shesha-reactjs/src/designer-components/numberField/numberField.tsx(1 hunks)shesha-reactjs/src/utils/style.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- shesha-reactjs/src/designer-components/numberField/numberField.tsx
- shesha-reactjs/src/designer-components/_settings/utils/dimensions/utils.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx (7)
shesha-reactjs/src/providers/form/store/shaFormInstance.tsx (2)
formSettings(150-152)formMode(154-156)shesha-reactjs/src/providers/formDesigner/index.tsx (1)
useFormDesignerStateSelector(370-370)shesha-reactjs/src/providers/canvas/index.tsx (1)
useCanvas(146-146)shesha-reactjs/src/components/formDesigner/utils/componentTypeUtils.ts (1)
getComponentTypeInfo(8-16)shesha-reactjs/src/hooks/formComponentHooks.ts (1)
useFormComponentStyles(179-254)shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (3)
getComponentDimensions(15-45)getDeviceDimensions(47-55)getDeviceFlexBasis(57-61)shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (1)
createRootContainerStyle(16-50)
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx (3)
shesha-reactjs/src/providers/form/store/shaFormInstance.tsx (1)
settings(241-243)shesha-reactjs/src/components/formDesigner/components/styles.ts (1)
useStyles(3-37)shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (1)
getDeviceDimensions(47-55)
shesha-reactjs/src/utils/style.ts (1)
shesha-reactjs/src/utils/nullables.ts (1)
isDefined(2-4)
shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (1)
shesha-reactjs/src/components/formDesigner/utils/componentTypeUtils.ts (1)
ComponentTypeInfo(3-6)
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (1)
shesha-reactjs/src/utils/style.ts (2)
addPx(41-50)hasNumber(71-71)
🪛 GitHub Actions: shesha-reactjs-build
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx
[error] 10-10: ESLint: 'addPx' is defined but never used. Allowed unused vars must match /^_/u (no-unused-vars).
🪛 GitHub Check: build-attempt
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx
[failure] 125-125:
There should be no space after this paren
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx
[failure] 10-10:
'addPx' is defined but never used. Allowed unused vars must match /^_/u
[failure] 59-59:
Unexpected console statement. Only these console methods are allowed: warn, dir, timeLog, assert, clear, count, countReset, group, groupEnd, table, dirxml, error, groupCollapsed, Console, profile, profileEnd, timeStamp, context
[failure] 58-58:
A space is required before '}'
[failure] 58-58:
A space is required after '{'
shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts
[failure] 49-49:
Block must not be padded by blank lines
[failure] 48-48:
Missing trailing comma
[failure] 3-3:
'addPx' is defined but never used. Allowed unused vars must match /^_/u
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts
[failure] 1-1:
'React' is defined but never used. Allowed unused vars must match /^_/u
🔇 Additional comments (2)
shesha-reactjs/src/designer-components/button/button.tsx (2)
17-17: LGTM!The import is necessary for accessing the form mode to enable designer-aware styling.
27-27: LGTM!The hook is properly initialized and used to access the form mode for conditional styling.
| import { addPx } from '@/utils/style'; | ||
| import { getDeviceDimensions } from '../utils/dimensionUtils'; |
There was a problem hiding this comment.
Drop unused import.
addPx isn’t used; remove to fix ESLint build error.
-import { addPx } from '@/utils/style';
-import { getDeviceDimensions } from '../utils/dimensionUtils';
+import { getDeviceDimensions } from '../utils/dimensionUtils';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { addPx } from '@/utils/style'; | |
| import { getDeviceDimensions } from '../utils/dimensionUtils'; | |
| import { getDeviceDimensions } from '../utils/dimensionUtils'; |
🧰 Tools
🪛 GitHub Actions: shesha-reactjs-build
[error] 10-10: ESLint: 'addPx' is defined but never used. Allowed unused vars must match /^_/u (no-unused-vars).
🪛 GitHub Check: build-attempt
[failure] 10-10:
'addPx' is defined but never used. Allowed unused vars must match /^_/u
🤖 Prompt for AI Agents
In
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx
around lines 10 to 11, the import "addPx" is unused and causing an ESLint build
error; remove the unused import from the import list so only getDeviceDimensions
remains imported to resolve the lint failure.
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx
Outdated
Show resolved
Hide resolved
| const componentDimensions = getComponentDimensions(typeInfo, dimensionsStyles); | ||
|
|
||
| const renderComponentModel = useMemo(() => { | ||
| const deviceDimensions = getDeviceDimensions( stylingBoxMargin); |
There was a problem hiding this comment.
Lint: remove space after paren.
getDeviceDimensions( stylingBoxMargin) → getDeviceDimensions(stylingBoxMargin).
- const deviceDimensions = getDeviceDimensions( stylingBoxMargin);
+ const deviceDimensions = getDeviceDimensions(stylingBoxMargin);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const deviceDimensions = getDeviceDimensions( stylingBoxMargin); | |
| const deviceDimensions = getDeviceDimensions(stylingBoxMargin); |
🧰 Tools
🪛 GitHub Check: build-attempt
[failure] 125-125:
There should be no space after this paren
🤖 Prompt for AI Agents
In
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx
around line 125, the function call has an extra space after the opening
parenthesis: change getDeviceDimensions( stylingBoxMargin) to
getDeviceDimensions(stylingBoxMargin) by removing the space after '('; update
the line accordingly and run the linter/formatting to ensure no other spacing
issues remain.
shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts
Outdated
Show resolved
Hide resolved
| export const getDeviceDimensions = ( | ||
| stylingBoxMargin: CSSProperties | ||
| ): CSSProperties => { | ||
|
|
||
| return { | ||
| width: `calc(100% - ${stylingBoxMargin?.marginLeft || '0px'} - ${stylingBoxMargin?.marginRight || '0px'} )`, | ||
| height: `calc(100% - ${stylingBoxMargin?.marginTop || '0px'} - ${stylingBoxMargin?.marginBottom || '0px'} )`, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Fix lint: trailing comma and padded blank line.
Add trailing comma to params and remove the blank line before return.
-export const getDeviceDimensions = (
- stylingBoxMargin: CSSProperties
-): CSSProperties => {
-
+export const getDeviceDimensions = (
+ stylingBoxMargin: CSSProperties,
+): CSSProperties => {
return {
width: `calc(100% - ${stylingBoxMargin?.marginLeft || '0px'} - ${stylingBoxMargin?.marginRight || '0px'} )`,
height: `calc(100% - ${stylingBoxMargin?.marginTop || '0px'} - ${stylingBoxMargin?.marginBottom || '0px'} )`,
};
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getDeviceDimensions = ( | |
| stylingBoxMargin: CSSProperties | |
| ): CSSProperties => { | |
| return { | |
| width: `calc(100% - ${stylingBoxMargin?.marginLeft || '0px'} - ${stylingBoxMargin?.marginRight || '0px'} )`, | |
| height: `calc(100% - ${stylingBoxMargin?.marginTop || '0px'} - ${stylingBoxMargin?.marginBottom || '0px'} )`, | |
| }; | |
| }; | |
| export const getDeviceDimensions = ( | |
| stylingBoxMargin: CSSProperties, | |
| ): CSSProperties => { | |
| return { | |
| width: `calc(100% - ${stylingBoxMargin?.marginLeft || '0px'} - ${stylingBoxMargin?.marginRight || '0px'} )`, | |
| height: `calc(100% - ${stylingBoxMargin?.marginTop || '0px'} - ${stylingBoxMargin?.marginBottom || '0px'} )`, | |
| }; | |
| }; |
🧰 Tools
🪛 GitHub Check: build-attempt
[failure] 49-49:
Block must not be padded by blank lines
[failure] 48-48:
Missing trailing comma
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts around
lines 47 to 55, the function signature is missing a trailing comma after the
parameter list and there's an extra blank line before the return statement;
remove the blank line immediately before the return and append a trailing comma
to the parameter declaration (after CSSProperties) so the params are properly
terminated.
| import React, { CSSProperties } from 'react'; | ||
| import { ComponentTypeInfo } from './componentTypeUtils'; | ||
| import { addPx, hasNumber } from '@/utils/style'; |
There was a problem hiding this comment.
Remove unused imports; use type‑only import for CSSProperties.
React and ComponentTypeInfo aren’t used; import CSSProperties as a type to satisfy ESLint/TS.
-import React, { CSSProperties } from 'react';
-import { ComponentTypeInfo } from './componentTypeUtils';
-import { addPx, hasNumber } from '@/utils/style';
+import type { CSSProperties } from 'react';
+import { addPx, canAddToCalc } from '@/utils/style';Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: build-attempt
[failure] 1-1:
'React' is defined but never used. Allowed unused vars must match /^_/u
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts around lines
1 to 3, remove the unused imports and make CSSProperties a type-only import:
delete the unused React default import and the unused ComponentTypeInfo import,
and change the React import so you import only the CSSProperties type (use
"import type { CSSProperties } from 'react'") to satisfy ESLint/TypeScript.
| const margins = { | ||
| marginTop: addPx(stylingBox?.marginTop || 0), | ||
| marginBottom: addPx(stylingBox?.marginBottom || 0), | ||
| marginLeft: addPx(stylingBox?.marginLeft || 0), | ||
| marginRight: addPx(stylingBox?.marginRight || 0), | ||
| }; |
There was a problem hiding this comment.
Avoid invalid calc() when margins are 'auto'/'none'; prefer capability check over digit check.
Building calc(...) + margin with 'auto'/'none' is invalid. Compute safe margins and gate on canAddToCalc (instead of hasNumber).
- const margins = {
- marginTop: addPx(stylingBox?.marginTop || 0),
- marginBottom: addPx(stylingBox?.marginBottom || 0),
- marginLeft: addPx(stylingBox?.marginLeft || 0),
- marginRight: addPx(stylingBox?.marginRight || 0),
- };
+ const rawTop = stylingBox?.marginTop ?? 0;
+ const rawBottom = stylingBox?.marginBottom ?? 0;
+ const rawLeft = stylingBox?.marginLeft ?? 0;
+ const rawRight = stylingBox?.marginRight ?? 0;
+ const margins = {
+ marginTop: canAddToCalc(rawTop) ? addPx(rawTop)! : '0px',
+ marginBottom: canAddToCalc(rawBottom) ? addPx(rawBottom)! : '0px',
+ marginLeft: canAddToCalc(rawLeft) ? addPx(rawLeft)! : '0px',
+ marginRight: canAddToCalc(rawRight) ? addPx(rawRight)! : '0px',
+ };
@@
- width: hasNumber(dimensions.width) ? `calc(${dimensions.width} + ${marginLeft} + ${marginRight})` : dimensions.width,
- maxWidth: hasNumber(dimensions.maxWidth) ? `calc(${dimensions.maxWidth} + ${marginLeft} + ${marginRight})` : dimensions.maxWidth,
- minWidth: hasNumber(dimensions.minWidth) ? `calc(${dimensions.minWidth} + ${marginLeft} + ${marginRight})` : dimensions.minWidth,
- height: hasNumber(dimensions.height) ? `calc(${dimensions.height} + ${marginTop} + ${marginBottom})` : dimensions.height,
- minHeight: hasNumber(dimensions.minHeight) ? `calc(${dimensions.minHeight} + ${marginTop} + ${marginBottom})` : dimensions.minHeight,
- maxHeight: hasNumber(dimensions.maxHeight) ? `calc(${dimensions.maxHeight} + ${marginTop} + ${marginBottom})` : dimensions.maxHeight,
+ width: canAddToCalc(dimensions.width as any) ? `calc(${dimensions.width} + ${marginLeft} + ${marginRight})` : dimensions.width,
+ maxWidth: canAddToCalc(dimensions.maxWidth as any) ? `calc(${dimensions.maxWidth} + ${marginLeft} + ${marginRight})` : dimensions.maxWidth,
+ minWidth: canAddToCalc(dimensions.minWidth as any) ? `calc(${dimensions.minWidth} + ${marginLeft} + ${marginRight})` : dimensions.minWidth,
+ height: canAddToCalc(dimensions.height as any) ? `calc(${dimensions.height} + ${marginTop} + ${marginBottom})` : dimensions.height,
+ minHeight: canAddToCalc(dimensions.minHeight as any) ? `calc(${dimensions.minHeight} + ${marginTop} + ${marginBottom})` : dimensions.minHeight,
+ maxHeight: canAddToCalc(dimensions.maxHeight as any) ? `calc(${dimensions.maxHeight} + ${marginTop} + ${marginBottom})` : dimensions.maxHeight,Also applies to: 39-47
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts around lines
25 to 30 (and similarly for lines 39 to 47), the code builds margin strings that
may perform calc(...) arithmetic against values like 'auto' or 'none', producing
invalid CSS; update the logic to first check canAddToCalc for each stylingBox
margin value and only wrap numeric values with addPx/calc logic, otherwise pass
through the original string (or a safe default) so you never concatenate calc
expressions with 'auto'/'none'; replace uses of hasNumber with canAddToCalc and
gate the addPx/calc operations accordingly for all
marginTop/marginBottom/marginLeft/marginRight assignments.
| export const canAddToCalc = (dimensionValue: string | number | undefined): boolean => { | ||
| if (!dimensionValue) return false; | ||
|
|
||
| const parsed = parseDimension(dimensionValue); | ||
| if (!parsed) return false; | ||
|
|
||
| // Auto and none cannot be used in calc with additions | ||
| if (parsed.unit === 'auto' || parsed.unit === 'none') { | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Broaden canAddToCalc to support calc() and CSS variables.
Values like calc(…) and var(…) are valid inside calc(...). Allow them while still excluding auto/none.
-export const canAddToCalc = (dimensionValue: string | number | undefined): boolean => {
+export const canAddToCalc = (dimensionValue: string | number | undefined): boolean => {
if (!dimensionValue) return false;
const parsed = parseDimension(dimensionValue);
- if (!parsed) return false;
+ // Allow calc(...) and var(...) tokens
+ if (!parsed) {
+ if (typeof dimensionValue === 'string') {
+ const v = dimensionValue.trim();
+ if (v.startsWith('calc(') || v.startsWith('var(')) return true;
+ }
+ return false;
+ }
// Auto and none cannot be used in calc with additions
if (parsed.unit === 'auto' || parsed.unit === 'none') {
return false;
}
return true;
};🤖 Prompt for AI Agents
In shesha-reactjs/src/utils/style.ts around lines 57-69, broaden canAddToCalc to
allow CSS functional values: first trim the input and if it matches a function
call for calc(...) or var(...) (case-insensitive, e.g. /^\s*(calc|var)\s*\(/)
return true (unless the string literally contains "auto" or "none"); otherwise
keep the existing parseDimension flow and continue to exclude parsed.unit ===
'auto' or 'none'. Implement the function-check before calling parseDimension so
calc(...) and var(...) are accepted while still blocking 'auto'/'none'.
- Updated ShaFormStyles to ensure components have a height of 100%. - Refactored ConfigurableFormItemLive to utilize updated useStyles for better label handling. - Improved dimension calculations in getDeviceDimensions to provide consistent 100% sizing. - Adjusted createRootContainerStyle to correctly apply margins as padding for better layout control. - Minor updates to button component for consistent height in designer mode.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
shesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsx (1)
43-43: Avoidanytype per coding guidelines.The
React.ReactElement<any>violates the project's coding guidelines which require eliminatinganyin favor ofunknownor proper types.Consider using a more specific type or
unknown:- const childrenElement = children as React.ReactElement<any>; + const childrenElement = children as React.ReactElement<unknown>;Based on coding guidelines requiring elimination of
anytype.shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx (1)
37-37: Avoidanytype per coding guidelines.
MutableRefObject<any>violates the project's coding guidelines. Consider using a more specific type orunknown.- settingsPanelRef?: MutableRefObject<any>; + settingsPanelRef?: MutableRefObject<HTMLElement | null>;Based on coding guidelines requiring elimination of
anytype.
♻️ Duplicate comments (2)
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx (1)
10-11: Drop unusedaddPximport.The
addPximport is not used in this file. This was flagged in a previous review.-import { addPx } from '@/utils/style'; -import { getDeviceDimensions } from '../utils/dimensionUtils'; +import { getDeviceDimensions } from '../utils/dimensionUtils';shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (1)
21-78: Refactor to eliminate implicitanytype and use proper validation for CSS values.Three issues require attention:
Line 89 - JSON.parse() returns implicit
anytype (violates coding guidelines): Changeconst parsed = JSON.parse(stylingBox);toconst parsed: unknown = JSON.parse(stylingBox);and add explicit type checking before spreading.Lines 38, 42, 46, 50, 54, 58 -
hasNumber()validation is insufficient: The current check only verifies digit presence (/\d/.test()) and cannot safely detect values like'auto'or'none'that will breakcalc()expressions. Use the existingcanAddToCalc()utility from@/utils/styleinstead, which properly handles CSS special keywords and units.Line 63 - Use type guard instead of
as constcasting: Apply proper TypeScript typing toboxSizingrather than type assertion.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
shesha-reactjs/src/components/configurableForm/styles/styles.ts(1 hunks)shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx(3 hunks)shesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsx(2 hunks)shesha-reactjs/src/components/formDesigner/components/styles.ts(1 hunks)shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx(7 hunks)shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts(1 hunks)shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts(1 hunks)shesha-reactjs/src/designer-components/button/button.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)
**/*.{ts,tsx}: Eliminate theanytype; useunknowntype instead for values with unknown types, forcing explicit type checking
Prefer type guards over type casting for type checking
Avoid monolithic types; use discriminated unions with a discriminator property instead
Leverage TypeScript to its full potential as a type system, not merely as a linter
Files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.tsshesha-reactjs/src/components/formDesigner/components/styles.tsshesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsxshesha-reactjs/src/components/configurableForm/styles/styles.tsshesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsxshesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsxshesha-reactjs/src/designer-components/button/button.tsxshesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts
🧠 Learnings (18)
📚 Learning: 2025-12-11T11:57:55.470Z
Learnt from: czwe-01
Repo: shesha-io/shesha-framework PR: 4349
File: shesha-reactjs/src/designer-components/columns/styles.ts:3-14
Timestamp: 2025-12-11T11:57:55.470Z
Learning: In the shesha-framework, when using createStyles to define styles hooks (e.g., export const useStyles = createStyles(...)), the standard pattern is to return an object with named properties (e.g., { columns }) from the createStyles callback, and then consume it in components using const { styles } = useStyles() followed by accessing the properties via styles.propertyName (e.g., styles.columns). The createStyles utility handles the wrapping automatically.
Applied to files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.tsshesha-reactjs/src/components/formDesigner/components/styles.tsshesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsxshesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsxshesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsx
📚 Learning: 2025-06-26T19:59:22.872Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:1280-1280
Timestamp: 2025-06-26T19:59:22.872Z
Learning: In shesha-reactjs chart settings forms, the styling panels (border, shadow, background) should not include buttonType-based hidden conditions since charts are not button components. The border configuration should follow the standard component pattern without button-specific restrictions.
Applied to files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.tsshesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsxshesha-reactjs/src/components/configurableForm/styles/styles.tsshesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsxshesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsxshesha-reactjs/src/designer-components/button/button.tsx
📚 Learning: 2025-10-28T14:21:55.782Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4084
File: shesha-reactjs/src/components/quickView/styles/styles.ts:41-94
Timestamp: 2025-10-28T14:21:55.782Z
Learning: In shesha-reactjs/src/components/quickView/styles/styles.ts, the quickViewContent styles intentionally use multiple `!important` flags and broad selectors (including spans, form items, inputs, links, and labels) to ensure proper targeting and prevent layout breakage in the quick view component.
Applied to files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.tsshesha-reactjs/src/components/formDesigner/components/styles.tsshesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsxshesha-reactjs/src/components/configurableForm/styles/styles.tsshesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsxshesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsxshesha-reactjs/src/designer-components/button/button.tsx
📚 Learning: 2025-05-28T07:55:21.036Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3312
File: shesha-reactjs/src/hooks/formComponentHooks.ts:183-192
Timestamp: 2025-05-28T07:55:21.036Z
Learning: In shesha-reactjs formComponentHooks.ts, the background style initialization deliberately sets valid values from properties panel immediately to provide better UX, rather than waiting for the useDeepCompareEffect to run. This intentional pattern prevents a flash of no background and shows the background image immediately when the component renders.
Applied to files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.tsshesha-reactjs/src/components/formDesigner/components/styles.tsshesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsxshesha-reactjs/src/components/configurableForm/styles/styles.tsshesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsxshesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsxshesha-reactjs/src/designer-components/button/button.tsx
📚 Learning: 2025-06-26T20:01:48.838Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:171-192
Timestamp: 2025-06-26T20:01:48.838Z
Learning: In shesha-reactjs chart settings forms, duplicate input configurations may be intentionally included as part of UI/UX experiments and should be flagged for review during UI/UX review phases rather than immediately removed as errors.
Applied to files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.tsshesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsxshesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsxshesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsx
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*.{ts,tsx} : Leverage TypeScript to its full potential as a type system, not merely as a linter
Applied to files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.
Applied to files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.tsshesha-reactjs/src/components/formDesigner/components/styles.tsshesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsxshesha-reactjs/src/components/configurableForm/styles/styles.tsshesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsxshesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsxshesha-reactjs/src/designer-components/button/button.tsx
📚 Learning: 2025-11-05T07:46:22.081Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/components/layerEditor/modal.tsx:55-57
Timestamp: 2025-11-05T07:46:22.081Z
Learning: In shesha-reactjs layer editor modal (src/components/layerEditor/modal.tsx), the useDeepCompareEffect that syncs items to the parent via onChange should only depend on [items], not [items, onChange]. Adding onChange to the dependency array causes the effect to fire on every parent render and breaks functionality. The guard for optional onChange is needed but the function reference should not be in the dependency array.
Applied to files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.tsshesha-reactjs/src/components/formDesigner/components/styles.tsshesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsxshesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsxshesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsx
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*{Component,Editor}.{ts,tsx} : Remove the `defaultValue` pattern entirely from controlled editor components
Applied to files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.tsshesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*.{ts,tsx} : Eliminate the `any` type; use `unknown` type instead for values with unknown types, forcing explicit type checking
Applied to files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*.{ts,tsx} : Avoid monolithic types; use discriminated unions with a discriminator property instead
Applied to files:
shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts
📚 Learning: 2025-11-20T14:51:46.152Z
Learnt from: AlexStepantsov
Repo: shesha-io/shesha-framework PR: 4235
File: shesha-reactjs/src/components/modelConfigurator/propertiesEditor/provider/reducer.ts:149-149
Timestamp: 2025-11-20T14:51:46.152Z
Learning: In shesha-reactjs model configurator reducer (src/components/modelConfigurator/propertiesEditor/provider/reducer.ts), when handling ArrayFormats.childObjects in the UpdateItem action, itemsType.dataFormat should be set to undefined (not ObjectFormats.object). This is the correct logic for child object array configuration.
Applied to files:
shesha-reactjs/src/components/formDesigner/components/styles.tsshesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsxshesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsxshesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsx
📚 Learning: 2025-11-05T08:12:08.149Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/providers/layersProvider/index.tsx:35-39
Timestamp: 2025-11-05T08:12:08.149Z
Learning: In shesha-reactjs LayerGroupConfiguratorProvider (src/providers/layersProvider/index.tsx), the provider uses a unidirectional data flow pattern after initialization: props.items provide initial state to the reducer (line 35-38), user actions modify the reducer state internally, and changes sync back to the parent via onChange callbacks in consuming components (e.g., modal.tsx line 55-57 uses useDeepCompareEffect). Do NOT add a useEffect to sync props.items back to the reducer state, as this creates a feedback loop that breaks layer updates in the Calendar UI. The pattern is intentionally "controlled initialization, uncontrolled updates" where the provider manages state internally after the initial mount.
Applied to files:
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsxshesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsxshesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsx
📚 Learning: 2025-10-02T11:25:33.370Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3926
File: shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts:138-166
Timestamp: 2025-10-02T11:25:33.370Z
Learning: In the shesha-framework repository, the hint popover components (sha-quick-search-hint-popover, sha-table-view-selector-hint-popover, sha-table-pager-hint-popover) in shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts intentionally use hard-coded color `rgb(214, 214, 214)` with `!important` to impose a specific consistent color across themes, overriding theme tokens by design.
Applied to files:
shesha-reactjs/src/components/configurableForm/styles/styles.tsshesha-reactjs/src/designer-components/button/button.tsx
📚 Learning: 2025-11-05T09:03:47.960Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/components/calendar/configurable-actions/refresh-calendar.tsx:5-18
Timestamp: 2025-11-05T09:03:47.960Z
Learning: In shesha-reactjs calendar configurable actions (src/components/calendar/configurable-actions/refresh-calendar.tsx), the useRefreshCalendarAction hook registers a configurable action with an empty dependency array []. Do NOT add setRefreshTrigger to the dependency array, as it is not stable (re-created when state.refreshTrigger changes) and would cause a registration loop. The executer uses a functional update pattern setRefreshTrigger((prev) => prev + 1) which doesn't need the current closure, so the action only needs to be registered once on mount.
<!--
Applied to files:
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx
📚 Learning: 2025-10-21T10:57:14.087Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4027
File: shesha-reactjs/src/components/calendar/index.tsx:76-87
Timestamp: 2025-10-21T10:57:14.087Z
Learning: In shesha-reactjs calendar components, when using executeScript with allData to evaluate external date expressions, allData must be included in the useEffect dependency array because the scripts may reference form data or global state that changes over time, and the dates need to be re-evaluated when that context changes.
Applied to files:
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*{Component,Editor}.{ts,tsx} : Default values must be part of model initialization logic, not in the editor component; controlled editors should follow the standard `value` and `onChange` contract
Applied to files:
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx
📚 Learning: 2025-12-02T14:32:10.827Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4280
File: shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts:205-227
Timestamp: 2025-12-02T14:32:10.827Z
Learning: In shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts, the mockup components (filterButtonMockup and viewSelectorMockup) intentionally use hardcoded colors (#d9d9d9, #fafafa, #8c8c8c) to maintain consistent appearance across themes, overriding theme tokens by design.
Applied to files:
shesha-reactjs/src/designer-components/button/button.tsx
🔇 Additional comments (6)
shesha-reactjs/src/components/configurableForm/styles/styles.ts (1)
36-36: Verify scope ofheight: 100%application and parent container height.The
height: 100%rule at line 36 applies to all.sha-components-containerinstances universally, affecting both horizontal and vertical layouts. The nested.horizontalrule only controls flex display of the inner container, not the height of the outer one.While the commit message indicates universal 100% height is intentional ("ensure components have a height of 100%"), confirm that:
- The
.sha-formparent container has an explicit height defined for this to function correctly- This universal application does not cause unintended layout issues in vertical form layouts
shesha-reactjs/src/components/formDesigner/components/styles.ts (1)
1-42: Well-structured styles hook following project patterns.The implementation correctly follows the shesha-framework pattern for
createStyleshooks with proper typing for theUseStylesPropsinterface. The use of!importantflags for form item styling aligns with the project's established patterns for ensuring proper targeting.shesha-reactjs/src/components/formDesigner/utils/stylingUtils.ts (1)
85-100: Consider omitting unused catch parameter.TypeScript allows omitting the catch parameter entirely when it's not used.
try { const parsed = JSON.parse(stylingBox); return JSON.stringify({ ...parsed, marginTop: 0, marginBottom: 0, marginLeft: 0, marginRight: 0, }); - } catch { + } catch { return stylingBox; }Actually, the current syntax is already correct (parameterless catch). No change needed.
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx (1)
111-126: The parameterless call togetDeviceDimensions()at line 112 is intentional and correct. The function is explicitly designed to accept no parameters and returns fixed 100% dimensions, as documented in its JSDoc comment. No action required.shesha-reactjs/src/designer-components/button/button.tsx (1)
27-27: Remove null-check suggestion foruseShaFormInstanceusage — the hook's error-throwing behavior provides fail-fast protection.The
useShaFormInstance()hook throws an error if called outside aShaFormProvidercontext, guaranteeing thatshaFormis non-null when the hook returns successfully. AccessingshaForm.formModedirectly without null checks is safe and follows the intended design pattern. If null-safety is needed in edge cases where the form context might not exist, useuseShaFormInstanceOrUndefined()instead.Likely an incorrect or invalid review comment.
shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts (1)
47-56: LGTM!The function correctly returns 100% dimensions for device-wrapped components. The JSDoc clearly explains the purpose, and the implementation is straightforward.
|
|
||
| const { hideLabel, hidden } = model; | ||
| const hasLabel = !hideLabel && !!model.label; | ||
| const { styles } = useStyles({ layout: settings.layout, hasLabel }); |
There was a problem hiding this comment.
Potential null reference: settings.layout without optional chaining.
On line 32, settings = shaForm.settings may be undefined. Accessing settings.layout on line 52 without optional chaining could throw a runtime error.
Apply this fix:
- const { styles } = useStyles({ layout: settings.layout, hasLabel });
+ const { styles } = useStyles({ layout: settings?.layout, hasLabel });🤖 Prompt for AI Agents
In
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx
around line 52, settings may be undefined (assigned from shaForm.settings
earlier), so calling useStyles({ layout: settings.layout, hasLabel }) risks a
null reference; update the call to safely handle undefined by using optional
chaining or a default (e.g., useStyles({ layout: settings?.layout, hasLabel })
or const settings = shaForm.settings || {} before the call) so layout access
cannot throw.
| wrapperCol: hideLabel ? { span: 24 } : layout?.wrapperCol, | ||
| // layout: model.layout, this property appears to have been removed from the Ant component | ||
| name: model.context ? undefined : getFieldNameFromExpression(propName), | ||
| style: { marginBottom, marginRight, marginLeft, marginTop, width: isInDesigner ? `calc(100% - ${marginLeft} - ${marginRight})` : width, height: isInDesigner ? `calc(100% - ${marginBottom} - ${marginTop})` : height, minHeight, minWidth, maxHeight: maxHeight, maxWidth }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extract complex inline style to a named variable for readability.
Line 72 contains a very long, complex inline style object that's difficult to read and maintain. Consider extracting it to a computed variable.
+ const formItemStyle: React.CSSProperties = {
+ marginBottom,
+ marginRight,
+ marginLeft,
+ marginTop,
+ width: isInDesigner ? `calc(100% - ${marginLeft} - ${marginRight})` : width,
+ height: isInDesigner ? `calc(100% - ${marginBottom} - ${marginTop})` : height,
+ minHeight,
+ minWidth,
+ maxHeight,
+ maxWidth,
+ };
+
const formItemProps: FormItemProps = {
className: classNames(className, styles.formItem, settings.layout),
label: hideLabel ? null : model.label,
// ... other props
- style: { marginBottom, marginRight, marginLeft, marginTop, width: isInDesigner ? `calc(100% - ${marginLeft} - ${marginRight})` : width, height: isInDesigner ? `calc(100% - ${marginBottom} - ${marginTop})` : height, minHeight, minWidth, maxHeight: maxHeight, maxWidth },
+ style: formItemStyle,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| style: { marginBottom, marginRight, marginLeft, marginTop, width: isInDesigner ? `calc(100% - ${marginLeft} - ${marginRight})` : width, height: isInDesigner ? `calc(100% - ${marginBottom} - ${marginTop})` : height, minHeight, minWidth, maxHeight: maxHeight, maxWidth }, | |
| const formItemStyle: React.CSSProperties = { | |
| marginBottom, | |
| marginRight, | |
| marginLeft, | |
| marginTop, | |
| width: isInDesigner ? `calc(100% - ${marginLeft} - ${marginRight})` : width, | |
| height: isInDesigner ? `calc(100% - ${marginBottom} - ${marginTop})` : height, | |
| minHeight, | |
| minWidth, | |
| maxHeight, | |
| maxWidth, | |
| }; | |
| const formItemProps: FormItemProps = { | |
| className: classNames(className, styles.formItem, settings.layout), | |
| label: hideLabel ? null : model.label, | |
| style: formItemStyle, | |
| }; |
🤖 Prompt for AI Agents
In
shesha-reactjs/src/components/formDesigner/components/configurableFormItemLive.tsx
around line 72, the long inline style object should be extracted to a named
variable to improve readability and maintainability; create a descriptive const
(e.g., containerStyle or computedStyle) above the JSX that builds the object
using the existing properties (marginBottom, marginRight, marginLeft, marginTop,
width, height, minHeight, minWidth, maxHeight, maxWidth) and the isInDesigner
conditional expressions (using template literals for calc(...) when needed),
optionally wrap the computation in useMemo with appropriate deps ([isInDesigner,
marginLeft, marginRight, marginTop, marginBottom, width, height, minHeight,
minWidth, maxHeight, maxWidth]) and then replace the inline style with
style={containerStyle} in the JSX.
| const childrenFunc = children as IConfigurableFormItemChildFunc; | ||
| return ( | ||
| <Form.Item {...formProps}> | ||
| <Form.Item {...formProps} className={styles.formItem}> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Duplicate className prop assignment.
The className: styles.formItem is already included in formProps (line 29), so specifying it again in the JSX spread is redundant. The prop in {...formProps} will be applied, and then overwritten by the explicit className={styles.formItem}.
Remove the duplicate:
- <Form.Item {...formProps} className={styles.formItem}>
+ <Form.Item {...formProps}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Form.Item {...formProps} className={styles.formItem}> | |
| <Form.Item {...formProps}> |
🤖 Prompt for AI Agents
In
shesha-reactjs/src/components/formDesigner/components/configurableFormItemSetting.tsx
around line 35, the Form.Item JSX spreads formProps which already contains
className: styles.formItem (set on line 29) and then redundantly sets
className={styles.formItem} again; remove the explicit
className={styles.formItem} from the JSX so the single className from
{...formProps} is used and there is no duplicate/overwritten prop.
| `); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove extra blank line.
Minor formatting cleanup - there's an unnecessary blank line before the closing backtick.
}
}
-
-
`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `); | |
| `); |
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/formDesigner/components/styles.ts around lines
36 to 38 there is an extra blank line before the closing template backtick;
remove that blank line so the template literal closes immediately after the last
style content (ensure no unintended whitespace/newline remains), then run
formatting/linting (prettier/ESLint) to confirm consistent styling.
| const rootContainerStyle = useMemo(() => | ||
| createRootContainerStyle(componentDimensions, margins, component.isInput), | ||
| [componentDimensions, margins, component.isInput] |
There was a problem hiding this comment.
Potential null reference: component.isInput without null check.
getToolboxComponent(componentModel?.type) on line 54 could return undefined if the component type doesn't exist. Accessing component.isInput on line 130 would throw in that case.
Add a fallback:
const rootContainerStyle = useMemo(() =>
- createRootContainerStyle(componentDimensions, margins, component.isInput),
- [componentDimensions, margins, component.isInput]
+ createRootContainerStyle(componentDimensions, margins, component?.isInput ?? false),
+ [componentDimensions, margins, component?.isInput]
);| <div style={{ | ||
| width: '100%', | ||
| height: '100%', | ||
| boxSizing: 'border-box', | ||
| }} | ||
| > | ||
| <DragWrapper componentId={componentModel.id} readOnly={readOnly}> | ||
| <div style={{ padding: '5px 3px' }}> | ||
| <FormComponent componentModel={componentModel} /> | ||
| <div style={{ | ||
| width: '100%', | ||
| height: '100%', | ||
| boxSizing: 'border-box', | ||
| }} | ||
| > | ||
| <FormComponent | ||
| componentModel={renderComponentModel} | ||
| /> | ||
| </div> | ||
| </DragWrapper> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider consolidating nested wrapper divs with identical styles.
Two wrapper divs (lines 170-175 and 177-182) have identical inline styles (width: 100%, height: 100%, boxSizing: 'border-box'). If the outer wrapper's only purpose is sizing, consider combining them or extracting the repeated style to a constant.
const innerWrapperStyle: React.CSSProperties = {
width: '100%',
height: '100%',
boxSizing: 'border-box',
};
// Then use:
<div style={innerWrapperStyle}>
<DragWrapper componentId={componentModel.id} readOnly={readOnly}>
<FormComponent componentModel={renderComponentModel} />
</DragWrapper>
</div>🤖 Prompt for AI Agents
In
shesha-reactjs/src/components/formDesigner/configurableFormComponent/index.tsx
around lines 170 to 188, two nested divs use identical inline styles (width:
'100%', height: '100%', boxSizing: 'border-box'); remove the redundant wrapper
by keeping a single wrapper around DragWrapper or extract the repeated style
into a shared constant (e.g., innerWrapperStyle) and apply it to the remaining
div to avoid duplication and improve clarity.
| export interface DimensionConfig { | ||
| width?: string; | ||
| height?: string; | ||
| maxWidth?: string; | ||
| minWidth?: string; | ||
| maxHeight?: string; | ||
| minHeight?: string; | ||
| flexBasis?: string; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider aligning dimension types with CSSProperties.
The DimensionConfig interface restricts all dimension properties to string, but CSSProperties allows string | number for these properties. This could cause type mismatches or require unnecessary type assertions when interoperating with standard CSS typing.
Apply this diff to align with CSSProperties:
export interface DimensionConfig {
- width?: string;
- height?: string;
- maxWidth?: string;
- minWidth?: string;
- maxHeight?: string;
- minHeight?: string;
- flexBasis?: string;
+ width?: string | number;
+ height?: string | number;
+ maxWidth?: string | number;
+ minWidth?: string | number;
+ maxHeight?: string | number;
+ minHeight?: string | number;
+ flexBasis?: string | number;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface DimensionConfig { | |
| width?: string; | |
| height?: string; | |
| maxWidth?: string; | |
| minWidth?: string; | |
| maxHeight?: string; | |
| minHeight?: string; | |
| flexBasis?: string; | |
| } | |
| export interface DimensionConfig { | |
| width?: string | number; | |
| height?: string | number; | |
| maxWidth?: string | number; | |
| minWidth?: string | number; | |
| maxHeight?: string | number; | |
| minHeight?: string | number; | |
| flexBasis?: string | number; | |
| } |
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts around
lines 5 to 13, the DimensionConfig interface currently types all dimension
properties as string which diverges from React CSSProperties (string | number).
Update the interface so each property accepts string | number (or import React
CSSProperties and use CSSProperties['width'] etc.) to match standard CSS typing
and avoid type mismatches when interoperating with style objects.
| const getDimensionValue = (dimensionType: keyof DimensionConfig): string | number => { | ||
| if (isDataTableContext) return '100%'; | ||
| return dimensionsStyles?.[dimensionType]; | ||
| }; |
There was a problem hiding this comment.
Fix return type to include undefined.
The getDimensionValue helper declares a return type of string | number, but dimensionsStyles?.[dimensionType] can be undefined. The function should explicitly include undefined in its return type to maintain type safety.
Apply this diff to fix the return type:
- const getDimensionValue = (dimensionType: keyof DimensionConfig): string | number => {
+ const getDimensionValue = (dimensionType: keyof DimensionConfig): string | number | undefined => {
if (isDataTableContext) return '100%';
return dimensionsStyles?.[dimensionType];
};As per coding guidelines, this leverages TypeScript's type system to prevent potential runtime errors.
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts around
lines 29 to 32, the getDimensionValue function is declared to return string |
number but may return undefined from dimensionsStyles?.[dimensionType]; update
the function signature to include undefined (string | number | undefined) so the
type accurately reflects possible outputs and adjust any callers if necessary to
handle the undefined case.
| export const getDeviceFlexBasis = ( | ||
| dimensionsStyles: CSSProperties, | ||
| ): string | number => { | ||
| return dimensionsStyles?.width; | ||
| }; |
There was a problem hiding this comment.
Fix return type to include undefined.
The function declares a return type of string | number, but dimensionsStyles?.width can be undefined. The return type should explicitly include undefined to accurately reflect the possible return values.
Apply this diff to fix the return type:
export const getDeviceFlexBasis = (
dimensionsStyles: CSSProperties,
-): string | number => {
+): string | number | undefined => {
return dimensionsStyles?.width;
};As per coding guidelines, this ensures TypeScript's type system is leveraged to its full potential.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getDeviceFlexBasis = ( | |
| dimensionsStyles: CSSProperties, | |
| ): string | number => { | |
| return dimensionsStyles?.width; | |
| }; | |
| export const getDeviceFlexBasis = ( | |
| dimensionsStyles: CSSProperties, | |
| ): string | number | undefined => { | |
| return dimensionsStyles?.width; | |
| }; |
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/formDesigner/utils/dimensionUtils.ts around
lines 58 to 62, the function getDeviceFlexBasis returns dimensionsStyles?.width
which can be undefined, but its signature declares string | number; update the
return type to string | number | undefined by modifying the function signature
accordingly so the type accurately reflects possible values.
| Factory: ({ model, form }) => { | ||
| const shaForm = useShaFormInstance(); | ||
| const { style, ...restProps } = model; | ||
|
|
||
| const finalStyle = { | ||
| ...model.allStyles.dimensionsStyles, | ||
| ...shaForm.formMode === 'designer' ? { width: '100%', height: '100%'}: model.allStyles.dimensionsStyles, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Spacing inconsistency in ternary expression.
Minor formatting issue: missing space before the colon in the ternary on line 31.
Apply this diff to fix the formatting:
- ...shaForm.formMode === 'designer' ? { width: '100%', height: '100%'}: model.allStyles.dimensionsStyles,
+ ...shaForm.formMode === 'designer' ? { width: '100%', height: '100%' } : model.allStyles.dimensionsStyles,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Factory: ({ model, form }) => { | |
| const shaForm = useShaFormInstance(); | |
| const { style, ...restProps } = model; | |
| const finalStyle = { | |
| ...model.allStyles.dimensionsStyles, | |
| ...shaForm.formMode === 'designer' ? { width: '100%', height: '100%'}: model.allStyles.dimensionsStyles, | |
| Factory: ({ model, form }) => { | |
| const shaForm = useShaFormInstance(); | |
| const { style, ...restProps } = model; | |
| const finalStyle = { | |
| ...shaForm.formMode === 'designer' ? { width: '100%', height: '100%' } : model.allStyles.dimensionsStyles, |
🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/button/button.tsx around lines 26 to
31, the ternary expression has inconsistent spacing (missing space before the
colon) causing a formatting/style issue; update the expression so the spacing
around the ternary operator is consistent (e.g., ensure there is a space before
and after the colon) and format the object spread line accordingly to match
project style guidelines.
- Introduced a componentsToSkip array to manage specific components that require different dimension handling. - Updated ConfigurableFormItemLive to conditionally adjust width and height based on the shouldSkip flag. - Enhanced getComponentTypeInfo to include shouldSkip logic for better component type management. - Refactored getComponentDimensions to utilize shouldSkip for consistent dimension calculations across components.
- Removed unnecessary null checks for gradient properties in getBackgroundStyle. - Enhanced readability by ensuring consistent spacing in gradient direction options. - Simplified dimensionsStyles calculation in useFormComponentStyles by removing unused dependencies.
…omponents - Introduced getCalculatedDimension utility for consistent width and height calculations in ConfigurableFormItemLive. - Updated default margins in ConfigurableFormItemLive to ensure proper spacing in designer mode. - Adjusted styles in various components to ensure full height and width where necessary, improving layout consistency. - Cleaned up unused styles and improved margin handling in styling utilities.
1 participant
Shesha Width Properties Affecting the wrong width #3557
Summary by CodeRabbit
New Features
Style
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.