Conversation
- Removed unnecessary margin from SectionSeparator. - Added preserveDimensionsInDesigner property to AttachmentsEditor. - Adjusted flexWrap logic in ContainerComponent based on flexDirection. - Removed default width and height from DateField when readOnly. - Cleaned up MultiColorInput by removing unused InputRow. - Updated RadioGroup to apply default margins for better spacing.
- Removed unused InputRow import from MultiColorInput. - Cleaned up the margin style in RadioGroup for improved readability.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughSmall UI and styling tweaks plus a new optional dimensions prop and designer flags: spacing and wrapper adjustments, read‑only sizing change, preserveDimensionsInDesigner flags added, dimensions prop threaded through table components into styling to emit height rules and adjust overflow. Changes
Sequence Diagram(s)mermaid Designer->>Wrapper: provide props (including dimensions) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shesha-reactjs/src/designer-components/container/containerComponent.tsx`:
- Line 64: The current assignment for flexWrap in containerComponent.tsx uses
model?.flexDirection?.includes('row') ? model.flexWrap : 'unset', which forces
'unset' when flexDirection is undefined; update the condition in the component
rendering logic (where flexWrap is computed) to treat an undefined/absent
model.flexDirection as row-default so that when model.flexWrap is provided it is
preserved (e.g., evaluate as true if model.flexDirection is undefined OR
includes('row')), otherwise fall back to 'unset' — adjust the expression that
computes flexWrap accordingly (reference: the flexWrap assignment using
model?.flexDirection?.includes('row') and model.flexWrap).
In `@shesha-reactjs/src/designer-components/radio/radioGroup.tsx`:
- Line 62: The Space component's inline style is appending "px" to
DEFAULT_MARGINS which already include units, causing duplicated units; update
the margin expression in radioGroup.tsx (where Space is rendered with
model.direction) to use the raw DEFAULT_MARGINS values without adding "px" (e.g.
margin: `${DEFAULT_MARGINS.vertical} ${DEFAULT_MARGINS.horizontal}`) so the CSS
receives valid unit-suffixed strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f72d6899-de99-4102-ae15-351a8a9b9715
📒 Files selected for processing (6)
shesha-reactjs/src/components/sectionSeparator/index.tsxshesha-reactjs/src/designer-components/attachmentsEditor/attachmentsEditor.tsxshesha-reactjs/src/designer-components/container/containerComponent.tsxshesha-reactjs/src/designer-components/dateField/dateField.tsxshesha-reactjs/src/designer-components/multiColorInput/index.tsxshesha-reactjs/src/designer-components/radio/radioGroup.tsx
💤 Files with no reviewable changes (2)
- shesha-reactjs/src/designer-components/dateField/dateField.tsx
- shesha-reactjs/src/components/sectionSeparator/index.tsx
shesha-reactjs/src/designer-components/container/containerComponent.tsx
Outdated
Show resolved
Hide resolved
- Simplified flexWrap assignment in ContainerComponent to directly use model.flexWrap. - Adjusted hidden condition in settingsForm to check for display instead of flexDirection. - Cleaned up margin style in RadioGroup for better consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shesha-reactjs/src/designer-components/dataTable/table/styles/styles.ts`:
- Around line 61-62: Remove the added overflow: auto from the .sha-react-table
rule so it no longer reintroduces overflow-x on the outer container; ensure
overflow-x remains applied only on the inner .sha-table (as intended by PR
`#4534`) and let the component-scoped overflow-y logic in the reactTable styles
(the dynamic overflow-y/boxShadow/freezeHeaders behavior) control vertical
overflow without being overridden by a global rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 63ac7e5a-9726-4bd1-8512-3ebc7600aa43
📒 Files selected for processing (1)
shesha-reactjs/src/designer-components/dataTable/table/styles/styles.ts
- Introduced dimensions prop in DataTable and ReactTable interfaces for better layout control. - Updated styles to accommodate dimensions for height, minHeight, and maxHeight. - Enhanced TableWrapper to pass dimensions prop for consistent behavior in the designer.
There was a problem hiding this comment.
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/components/reactTable/styles/styles.ts (1)
222-240:⚠️ Potential issue | 🟠 MajorRestore horizontal overflow on the inner table container.
The styling now applies dimension constraints, but horizontal scrolling is no longer guaranteed in this hook. In non-designer renders, this can remove horizontal scroll for wide tables.
💡 Proposed fix
.${shaTable} { border-spacing: 0; display: inline-block; min-width: 100%; + overflow-x: auto;Based on learnings: moving
overflow-x: autofrom the outer.sha-react-tablecontainer to the inner.sha-tableelement is intentional and works correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shesha-reactjs/src/components/reactTable/styles/styles.ts` around lines 222 - 240, The inner table container lost horizontal scrolling; add horizontal overflow back to the inner table element by ensuring the CSS for .${shaTable} includes overflow-x: auto (and overflow-y: visible/hidden as needed) so wide tables can scroll horizontally; locate the styles for the .${shaTable} block and restore/add the overflow-x rule there (rather than on the outer .sha-react-table) so non-designer renders regain horizontal scroll.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@shesha-reactjs/src/components/reactTable/styles/styles.ts`:
- Around line 222-240: The inner table container lost horizontal scrolling; add
horizontal overflow back to the inner table element by ensuring the CSS for
.${shaTable} includes overflow-x: auto (and overflow-y: visible/hidden as
needed) so wide tables can scroll horizontally; locate the styles for the
.${shaTable} block and restore/add the overflow-x rule there (rather than on the
outer .sha-react-table) so non-designer renders regain horizontal scroll.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 10765e32-727f-4965-ad8d-fb83b970b176
📒 Files selected for processing (6)
shesha-reactjs/src/components/dataTable/index.tsxshesha-reactjs/src/components/reactTable/index.tsxshesha-reactjs/src/components/reactTable/interfaces.tsshesha-reactjs/src/components/reactTable/styles/styles.tsshesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsxshesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx
#3557
Summary by CodeRabbit
Style
New Features
Bug Fixes