Conversation
Fixed column boundary misalignment between inline add row and table body by conditionally applying shaCrudCell styling only to crud-operations columns instead of all cells. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Moved rowHeight and rowBorder/rowBorderStyle from applying to all rows to only applying to table body rows. Row padding (effectivePadding) now only applies to body cells, not header cells. Fixes: - rowHeight now only affects .tr-body rows - rowBorder/rowBorderStyle now only affects .tr-body rows - rowPadding now only affects .td cells, not .th cells Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Enhanced the row click handler to prevent triggering On Row Click events when clicking on form inputs during inline editing. Added checks for: - Button elements (dropdowns, pickers) - Missing Ant Design components (checkbox, radio, switch, slider, rate, upload) - Form cell wrapper (.sha-form-cell) - All clicks when row is in edit mode Fixes issue where clicking inside input fields during inline editing incorrectly triggered the configured On Row Click event. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Enhanced row click handler to detect clicks inside Ant Design portaled content (dropdowns, pickers, popovers) that render outside the row DOM. Prevents On Row Click from firing when clicking on: - Select dropdowns (.ant-select-dropdown) - DatePicker/TimePicker calendars (.ant-picker-dropdown) - General dropdowns (.ant-dropdown) - Popovers, modals, drawers, tooltips Fixes issue where clicking dates in calendar or options in dropdown triggered row click for the row beneath the portal. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…r selectedRow.id access
Fixed On Row Double Click to provide correct context for Navigation and
Show Dialog actions. The context now matches single-click structure:
- Uses 'row' and 'rowIndex' keys (not 'data')
- Passes row.original (actual data) instead of react-table row object
- Includes selectedRow with proper structure: { index, row, id }
This ensures selectedRow.id is available in parameter configurations for
Navigation and Show Dialog actions triggered by double-click.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fixed table background color/image not showing through table rows. Changes: - Table container now uses configured backgroundColor instead of hardcoded white - Rows explicitly set to transparent when rowBackgroundColor is not configured - Striped rows explicitly set to transparent when no alternate background This allows the configured table background to be visible when rows have no background color set, instead of showing white. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Made inner table elements (.sha-table and .tbody) transparent so gradient and image backgrounds from the outer container can show through. Previously, these inner elements were setting their own background-color which blocked non-solid backgrounds. Background layering now works correctly: - Outer .sha-react-table: has the configured background (color/gradient/image) - Inner .sha-table: transparent (shows through) - Inner .tbody: transparent (shows through) - Rows .tr-body: transparent when no row color (shows through) Gradients and images now display without requiring table padding. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The table was only extracting background.color, ignoring gradients and images. Now uses getBackgroundStyle() to convert the full background object (color/gradient/url/image/storedFile) to proper CSS. Changes: - Import getBackgroundStyle from background utils - Add effectiveBackground useMemo to convert background object to CSS - Pass effectiveBackground to useStyles instead of background.color - Supports linear-gradient, radial-gradient, and url() backgrounds Works with transparent inner elements fix from previous commit to allow full background visibility. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
WalkthroughThis PR updates ReactTable's row double-click handler to pass rowIndex as a parameter, refines styling logic for CRUD-operation columns, extensively refactors table body styling to propagate font customization and padding properties, enhances row-click prevention for portal elements and editing states, and improves background style computation through memoization. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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
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
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/reactTable/styles/styles.ts`:
- Around line 325-334: The inner IIFE that handles row borders shadows the
outer-scope variable named borderStyles; update the inner variable name (used to
store the result of getBorderStyle(rowBorderStyle, {})) to a non-conflicting
identifier such as computedBorderStyles or localBorderStyles and adjust its
usage in the subsequent Object.entries(...).map(...) chain so references to the
inner result no longer reuse the outer borderStyles name; this change should be
made within the block that references rowBorderStyle, rowBorder and
getBorderStyle to preserve behavior while eliminating the shadowing.
- Around line 339-418: Repeated body font style template across useMainStyles
makes maintenance hard; add a helper function (e.g., getBodyFontStyles(important
= false)) inside useMainStyles after destructuring tableClassNames that returns
the concatenated font CSS for
bodyFontFamily/bodyFontSize/bodyFontWeight/bodyFontColor/bodyTextAlign with
optional " !important", then replace each repeated inline template block
(selectors like .${td}, .sha-data-cell, .sha-form-cell, .sha-form-cell
.ant-form-item-control-input-content, .ant-select-selection-item / .ant-input /
.sha-data-cell input etc., and .sha-input inner input) with a single call to
${getBodyFontStyles(true)} or ${getBodyFontStyles()} as appropriate to preserve
existing specificity.
| ${(() => { | ||
| // Prefer rowBorderStyle over rowBorder for full border control | ||
| if (rowBorderStyle) { | ||
| const borderStyles = getBorderStyle(rowBorderStyle, {}); | ||
| return Object.entries(borderStyles) | ||
| .map(([key, value]) => `${key.replace(/([A-Z])/g, '-$1').toLowerCase()}: ${value};`) | ||
| .join(' '); | ||
| } | ||
| return rowBorder ? `border: ${rowBorder};` : ''; | ||
| })()} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Variable shadowing: borderStyles shadows the outer-scope variable.
The variable borderStyles on line 328 shadows the borderStyles defined on line 140. While this works due to IIFE scoping, it reduces readability and could cause confusion during maintenance.
♻️ Suggested rename
${(() => {
// Prefer rowBorderStyle over rowBorder for full border control
if (rowBorderStyle) {
- const borderStyles = getBorderStyle(rowBorderStyle, {});
- return Object.entries(borderStyles)
+ const rowBorderStyles = getBorderStyle(rowBorderStyle, {});
+ return Object.entries(rowBorderStyles)
.map(([key, value]) => `${key.replace(/([A-Z])/g, '-$1').toLowerCase()}: ${value};`)
.join(' ');
}
return rowBorder ? `border: ${rowBorder};` : '';
})()}📝 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.
| ${(() => { | |
| // Prefer rowBorderStyle over rowBorder for full border control | |
| if (rowBorderStyle) { | |
| const borderStyles = getBorderStyle(rowBorderStyle, {}); | |
| return Object.entries(borderStyles) | |
| .map(([key, value]) => `${key.replace(/([A-Z])/g, '-$1').toLowerCase()}: ${value};`) | |
| .join(' '); | |
| } | |
| return rowBorder ? `border: ${rowBorder};` : ''; | |
| })()} | |
| ${(() => { | |
| // Prefer rowBorderStyle over rowBorder for full border control | |
| if (rowBorderStyle) { | |
| const rowBorderStyles = getBorderStyle(rowBorderStyle, {}); | |
| return Object.entries(rowBorderStyles) | |
| .map(([key, value]) => `${key.replace(/([A-Z])/g, '-$1').toLowerCase()}: ${value};`) | |
| .join(' '); | |
| } | |
| return rowBorder ? `border: ${rowBorder};` : ''; | |
| })()} |
🤖 Prompt for AI Agents
In `@shesha-reactjs/src/components/reactTable/styles/styles.ts` around lines 325 -
334, The inner IIFE that handles row borders shadows the outer-scope variable
named borderStyles; update the inner variable name (used to store the result of
getBorderStyle(rowBorderStyle, {})) to a non-conflicting identifier such as
computedBorderStyles or localBorderStyles and adjust its usage in the subsequent
Object.entries(...).map(...) chain so references to the inner result no longer
reuse the outer borderStyles name; this change should be made within the block
that references rowBorderStyle, rowBorder and getBorderStyle to preserve
behavior while eliminating the shadowing.
| /* Apply text alignment and font styles to body cells */ | ||
| .${td} { | ||
| ${bodyTextAlign ? `text-align: ${bodyTextAlign} !important;` : ''} | ||
| ${bodyFontFamily ? `font-family: ${bodyFontFamily} !important;` : ''} | ||
| ${bodyFontSize ? `font-size: ${bodyFontSize};` : ''} | ||
| ${bodyFontWeight ? `font-weight: ${bodyFontWeight};` : ''} | ||
| ${bodyFontColor ? `color: ${bodyFontColor};` : ''} | ||
| } | ||
|
|
||
| /* Apply body font styles to custom component wrappers */ | ||
| .sha-data-cell, | ||
| .sha-form-cell { | ||
| ${bodyFontFamily ? `font-family: ${bodyFontFamily} !important;` : ''} | ||
| ${bodyFontSize ? `font-size: ${bodyFontSize} !important;` : ''} | ||
| ${bodyFontWeight ? `font-weight: ${bodyFontWeight} !important;` : ''} | ||
| ${bodyFontColor ? `color: ${bodyFontColor} !important;` : ''} | ||
| ${bodyTextAlign ? `text-align: ${bodyTextAlign} !important;` : ''} | ||
|
|
||
| } | ||
|
|
||
| /* Apply body font styles to form component content */ | ||
| .sha-form-cell .ant-form-item-control-input-content { | ||
| ${bodyFontFamily ? `font-family: ${bodyFontFamily} !important;` : ''} | ||
| ${bodyFontSize ? `font-size: ${bodyFontSize} !important;` : ''} | ||
| ${bodyFontWeight ? `font-weight: ${bodyFontWeight} !important;` : ''} | ||
| ${bodyFontColor ? `color: ${bodyFontColor} !important;` : ''} | ||
| ${bodyTextAlign ? `text-align: ${bodyTextAlign} !important;` : ''} | ||
| } | ||
|
|
||
| /* Override Ant Design component defaults with table body font styles */ | ||
| .ant-select-selection-item, | ||
| .ant-select-selection-placeholder, | ||
| .ant-input, | ||
| .ant-input-number-input, | ||
| .ant-picker-input > input, | ||
| .sha-data-cell input, | ||
| .sha-data-cell .ant-select-selection-search-input { | ||
| ${bodyFontFamily ? `font-family: ${bodyFontFamily} !important;` : ''} | ||
| ${bodyFontSize ? `font-size: ${bodyFontSize} !important;` : ''} | ||
| ${bodyFontWeight ? `font-weight: ${bodyFontWeight} !important;` : ''} | ||
| ${bodyFontColor ? `color: ${bodyFontColor} !important;` : ''} | ||
| ${bodyTextAlign ? `text-align: ${bodyTextAlign} !important;` : ''} | ||
| } | ||
|
|
||
| /* Apply to autocomplete and entity reference components */ | ||
| .sha-autocomplete-raw-value, | ||
| .sha-entity-reference-display-name { | ||
| ${bodyFontFamily ? `font-family: ${bodyFontFamily} !important;` : ''} | ||
| ${bodyFontSize ? `font-size: ${bodyFontSize} !important;` : ''} | ||
| ${bodyFontWeight ? `font-weight: ${bodyFontWeight} !important;` : ''} | ||
| ${bodyFontColor ? `color: ${bodyFontColor} !important;` : ''} | ||
| ${bodyTextAlign ? `text-align: ${bodyTextAlign} !important;` : ''} | ||
| } | ||
|
|
||
| /* Apply to readonly display components */ | ||
| .read-only-display-form-item, | ||
| .read-only-display-form-item div { | ||
| ${bodyFontFamily ? `font-family: ${bodyFontFamily} !important;` : ''} | ||
| ${bodyFontSize ? `font-size: ${bodyFontSize} !important;` : ''} | ||
| ${bodyFontWeight ? `font-weight: ${bodyFontWeight} !important;` : ''} | ||
| ${bodyFontColor ? `color: ${bodyFontColor} !important;` : ''} | ||
| ${bodyTextAlign ? `text-align: ${bodyTextAlign} !important;` : ''} | ||
| } | ||
|
|
||
| /* Apply to text field components */ | ||
| .sha-input { | ||
| ${bodyFontFamily ? `font-family: ${bodyFontFamily} !important;` : ''} | ||
| ${bodyFontSize ? `font-size: ${bodyFontSize} !important;` : ''} | ||
| ${bodyFontWeight ? `font-weight: ${bodyFontWeight} !important;` : ''} | ||
| ${bodyFontColor ? `color: ${bodyFontColor} !important;` : ''} | ||
| ${bodyTextAlign ? `text-align: ${bodyTextAlign} !important;` : ''} | ||
| input { | ||
| ${bodyFontFamily ? `font-family: ${bodyFontFamily} !important;` : ''} | ||
| ${bodyFontSize ? `font-size: ${bodyFontSize} !important;` : ''} | ||
| ${bodyFontWeight ? `font-weight: ${bodyFontWeight} !important;` : ''} | ||
| ${bodyFontColor ? `color: ${bodyFontColor} !important;` : ''} | ||
| ${bodyTextAlign ? `text-align: ${bodyTextAlign} !important;` : ''} | ||
|
|
||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting repeated font style logic into a helper.
The same font styling properties (bodyFontFamily, bodyFontSize, bodyFontWeight, bodyFontColor, bodyTextAlign) are repeated across 10+ selectors (lines 340-346, 349-357, 360-366, 369-381, 384-391, 394-401, 404-417). This creates maintenance overhead when font properties need to change.
♻️ Suggested refactor: Extract a helper function
// Add this helper near the top of useMainStyles, after destructuring tableClassNames
const getBodyFontStyles = (important = false) => {
const imp = important ? ' !important' : '';
return `
${bodyFontFamily ? `font-family: ${bodyFontFamily}${imp};` : ''}
${bodyFontSize ? `font-size: ${bodyFontSize}${imp};` : ''}
${bodyFontWeight ? `font-weight: ${bodyFontWeight}${imp};` : ''}
${bodyFontColor ? `color: ${bodyFontColor}${imp};` : ''}
${bodyTextAlign ? `text-align: ${bodyTextAlign}${imp};` : ''}
`;
};
// Then replace repeated blocks with:
.sha-data-cell,
.sha-form-cell {
${getBodyFontStyles(true)}
}🤖 Prompt for AI Agents
In `@shesha-reactjs/src/components/reactTable/styles/styles.ts` around lines 339 -
418, Repeated body font style template across useMainStyles makes maintenance
hard; add a helper function (e.g., getBodyFontStyles(important = false)) inside
useMainStyles after destructuring tableClassNames that returns the concatenated
font CSS for
bodyFontFamily/bodyFontSize/bodyFontWeight/bodyFontColor/bodyTextAlign with
optional " !important", then replace each repeated inline template block
(selectors like .${td}, .sha-data-cell, .sha-form-cell, .sha-form-cell
.ant-form-item-control-input-content, .ant-select-selection-item / .ant-input /
.sha-data-cell input etc., and .sha-input inner input) with a single call to
${getBodyFontStyles(true)} or ${getBodyFontStyles()} as appropriate to preserve
existing specificity.
Summary by CodeRabbit
Bug Fixes
UI Improvements
✏️ Tip: You can customize this high-level summary in your review settings.