Conversation
WalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TableRow
participant ClickHandler
participant Timer as 250ms Timer
User->>TableRow: Click (first)
TableRow->>ClickHandler: schedule single-click (set Timer)
TableRow->>Timer: start 250ms
alt Second click within 250ms (double-click)
User->>TableRow: Click (second)
TableRow->>ClickHandler: mark double-click, clear Timer
ClickHandler->>TableRow: invoke onDoubleClick() (immediate)
else No second click
Timer->>ClickHandler: timer fires
ClickHandler->>TableRow: invoke onClick()/onRowClick()
end
sequenceDiagram
participant TableWrapper
participant getShadowStyle
participant DataTable
participant ReactTable
participant RowRenderer
TableWrapper->>TableWrapper: memoize props.shadow?
alt shadow provided
TableWrapper->>getShadowStyle: getShadowStyle(shadow)
getShadowStyle-->>TableWrapper: boxShadow
TableWrapper->>DataTable: finalBoxShadow, striped
else no shadow
TableWrapper-->>DataTable: boxShadow (from props), striped
end
DataTable->>ReactTable: pass striped, boxShadow
ReactTable->>RowRenderer: pass striped
RowRenderer->>RowRenderer: if striped && index%2==odd -> apply trOdd
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)
Files:
🧠 Learnings (9)📓 Common learnings📚 Learning: 2025-11-25T07:17:44.436ZApplied to files:
📚 Learning: 2025-11-05T07:46:22.081ZApplied to files:
📚 Learning: 2025-10-21T10:57:14.087ZApplied to files:
📚 Learning: 2025-11-05T09:03:47.960ZApplied to files:
📚 Learning: 2025-11-05T08:12:08.149ZApplied to files:
📚 Learning: 2025-11-25T07:17:44.436ZApplied to files:
📚 Learning: 2025-11-25T07:17:44.436ZApplied to files:
📚 Learning: 2025-11-20T14:51:46.128ZApplied to files:
⏰ 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)
🔇 Additional comments (6)
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
shesha-reactjs/src/components/dataTable/index.tsx(4 hunks)shesha-reactjs/src/components/reactTable/index.tsx(3 hunks)shesha-reactjs/src/components/reactTable/interfaces.ts(1 hunks)shesha-reactjs/src/components/reactTable/styles/styles.ts(4 hunks)shesha-reactjs/src/components/reactTable/tableRow.tsx(3 hunks)shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts(1 hunks)shesha-reactjs/src/designer-components/dataTable/table/models.ts(2 hunks)shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx(4 hunks)shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts(1 hunks)shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: HackGenesis
Repo: shesha-io/shesha-framework PR: 4094
File: shesha-reactjs/src/components/dataTable/index.tsx:971-977
Timestamp: 2025-10-30T13:25:02.065Z
Learning: In the shesha-framework DataTable/ReactTable components, the useMultiSelect prop controls whether the checkbox selection column is added to the table, not the selection behavior itself. The actual selection behavior (single vs multiple) is controlled by the selectionMode prop. For both 'single' and 'multiple' modes, useMultiSelect should be true to show checkboxes. The ReactTable component's onSingleRowToggle function (around line 314-316 in shesha-reactjs/src/components/reactTable/index.tsx) enforces single-selection by clearing all other selections when a new row is selected in single mode.
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.
📚 Learning: 2025-10-30T13:25:02.065Z
Learnt from: HackGenesis
Repo: shesha-io/shesha-framework PR: 4094
File: shesha-reactjs/src/components/dataTable/index.tsx:971-977
Timestamp: 2025-10-30T13:25:02.065Z
Learning: In the shesha-framework DataTable/ReactTable components, the useMultiSelect prop controls whether the checkbox selection column is added to the table, not the selection behavior itself. The actual selection behavior (single vs multiple) is controlled by the selectionMode prop. For both 'single' and 'multiple' modes, useMultiSelect should be true to show checkboxes. The ReactTable component's onSingleRowToggle function (around line 314-316 in shesha-reactjs/src/components/reactTable/index.tsx) enforces single-selection by clearing all other selections when a new row is selected in single mode.
Applied to files:
shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.tsshesha-reactjs/src/components/reactTable/index.tsxshesha-reactjs/src/components/reactTable/styles/styles.tsshesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsxshesha-reactjs/src/components/reactTable/tableRow.tsxshesha-reactjs/src/designer-components/dataTable/table/tableSettings.tsshesha-reactjs/src/components/reactTable/interfaces.tsshesha-reactjs/src/components/dataTable/index.tsx
📚 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/designer-components/dataTable/table/migrations/migrate-v13.tsshesha-reactjs/src/designer-components/dataTable/table/models.tsshesha-reactjs/src/components/reactTable/index.tsxshesha-reactjs/src/components/reactTable/styles/styles.tsshesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsxshesha-reactjs/src/designer-components/dataTable/table/tableSettings.tsshesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsxshesha-reactjs/src/components/reactTable/interfaces.tsshesha-reactjs/src/components/dataTable/index.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/designer-components/dataTable/table/models.tsshesha-reactjs/src/components/reactTable/index.tsxshesha-reactjs/src/components/reactTable/styles/styles.tsshesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsxshesha-reactjs/src/designer-components/dataTable/table/tableSettings.tsshesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsxshesha-reactjs/src/components/reactTable/interfaces.tsshesha-reactjs/src/components/dataTable/index.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/designer-components/dataTable/table/models.tsshesha-reactjs/src/components/reactTable/index.tsxshesha-reactjs/src/components/reactTable/styles/styles.tsshesha-reactjs/src/designer-components/dataTable/table/tableSettings.tsshesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsxshesha-reactjs/src/components/reactTable/interfaces.tsshesha-reactjs/src/components/dataTable/index.tsx
📚 Learning: 2025-06-26T20:00:58.574Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/chartControl.tsx:51-51
Timestamp: 2025-06-26T20:00:58.574Z
Learning: In shesha-reactjs chart components (ChartControl), filters are intentionally passed as props rather than obtained from context. This allows the query builder and other external components to dynamically update filters. The pattern is: most chart configuration comes from useChartDataStateContext(), but filters are passed as props from the chart factories (bar.tsx, line.tsx, pie.tsx, polarArea.tsx) for dynamic updates.
Applied to files:
shesha-reactjs/src/components/reactTable/index.tsxshesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsxshesha-reactjs/src/components/reactTable/interfaces.ts
📚 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/reactTable/index.tsxshesha-reactjs/src/components/reactTable/styles/styles.tsshesha-reactjs/src/components/reactTable/tableRow.tsxshesha-reactjs/src/designer-components/dataTable/table/tableSettings.tsshesha-reactjs/src/designer-components/dataTable/table/tableWrapper.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/reactTable/index.tsxshesha-reactjs/src/components/reactTable/styles/styles.tsshesha-reactjs/src/designer-components/dataTable/table/tableWrapper.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/designer-components/dataTable/table/tableSettings.tsshesha-reactjs/src/components/reactTable/interfaces.ts
📚 Learning: 2025-11-20T14:51:46.128Z
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.128Z
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/designer-components/dataTable/table/tableSettings.ts
📚 Learning: 2025-06-06T10:41:31.642Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3362
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:429-448
Timestamp: 2025-06-06T10:41:31.642Z
Learning: In shesha-reactjs chart settings, conditional visibility for controls is intentionally designed to show different combinations based on chart type: pie/polarArea charts show showTitle+showLegend together, while other chart types show showTitle separately. This pattern of "one shows when others doesn't show" is by design.
Applied to files:
shesha-reactjs/src/designer-components/dataTable/table/tableSettings.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 (11)
shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts (1)
940-940: LGTM! Consistent default value for striped styling.The default value aligns with the
striped = truedefaults in the ReactTable and DataTable components, ensuring consistency across the designer interface and runtime behavior.shesha-reactjs/src/designer-components/dataTable/table/models.ts (1)
7-7: LGTM! Shadow styling support added.The addition of the
shadowproperty follows the existing pattern for styling properties likeborderand integrates properly with the shadow processing in tableWrapper.tsx.Also applies to: 57-57
shesha-reactjs/src/components/reactTable/interfaces.ts (1)
253-253: LGTM! Property addition for striped styling.The
stripedproperty is properly added to the interface and is used consistently throughout the ReactTable implementation.shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx (1)
63-68: LGTM! Clean shadow processing implementation.The memoized shadow processing properly converts the shadow configuration object to a boxShadow string, with a clean fallback to the legacy boxShadow prop. The implementation is efficient and consistent throughout the component.
shesha-reactjs/src/components/reactTable/index.tsx (1)
109-109: LGTM! Striped styling properly integrated.The striped prop is defaulted to
trueand correctly propagated through the styling hook and row rendering. The implementation enables alternating row colors by default, which is a common and useful table pattern.Also applies to: 137-137, 616-616
shesha-reactjs/src/components/reactTable/styles/styles.ts (1)
57-57: LGTM! Striped parameter and tbody styling additions.The
stripedparameter is properly added to the styling hook signature and types. The conditional backgroundColor application to tbody is appropriate for providing customizable table backgrounds.Also applies to: 74-74, 190-190
shesha-reactjs/src/components/dataTable/index.tsx (2)
376-379: Verify the onSelectionChange behavior change is intentional.This change prevents
onSelectionChangefrom firing when transitioning from no selection to the first selection. While this may be intentional to avoid redundant callbacks, it represents a behavior change that could affect existing implementations expecting the callback on first selection.Consider:
- Documenting this behavior change in release notes as it may be breaking for some users
- Verifying that this aligns with the intended UX for the
onSelectionChangeevent- Confirming whether users should use
onRowSelectinstead for first-selection eventsIf this is part of addressing issue #2603, ensure the issue description captures this behavioral distinction.
80-80: LGTM! Striped prop properly integrated into DataTable.The
stripedproperty is correctly added to the interface, defaulted totrue, and properly passed through to the underlying ReactTable component.Also applies to: 138-138, 937-937
shesha-reactjs/src/components/reactTable/tableRow.tsx (1)
35-36: Striped row prop plumbing and conditional class look consistent
stripedis correctly added toISortableRowProps, passed throughTableRow/SortableRow, and used to guard the striped class via{ [styles.trOdd]: striped && index % 2 === 0 }. This keeps existing styling intact while allowing striped rows to be disabled per table.Also applies to: 77-78, 165-167
shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx (2)
9-9: New migration hook for v12→v13 is wired correctlyThe
migrateV12toV13import and its registration at step 13 in the migrator chain fit the existing migration pattern and sequencing, slotting in between style migration (step 12) and striped defaults (steps 14–15).Please confirm
migrate-v13only relies on fields that are stable after step 12 (e.g., selection-related props) and doesn’t need to run earlier in the chain.Also applies to: 120-122
46-52: Manual verification needed: migrator step 14 striped override behaviorThe codebase files are not accessible in the sandbox environment, so I cannot verify the actual implementation of migrator step 14. The concern raised—that step 14 unconditionally forces
striped: true, potentially overriding user-configuredstriped: false—cannot be confirmed or refuted through code inspection.However, note that similar migration patterns in chart components (e.g.,
BarChartComponent) intentionally override defaults by design. Verify whether the table component's step 14 follows the same intentional pattern, or if it's an unintended data loss issue.Please confirm:
- Is step 14 forcing
striped: trueby design to normalize legacy configurations, or is it an unintended override?- Do existing tables with
striped: falseneed to preserve that setting, or should migration default tostriped: true?
| min-width: 100%; | ||
| background-color: ${backgroundColor} | ||
| ${boxShadow ? `box-shadow: ${boxShadow};` : ''} | ||
| ${boxShadow ? `box-shadow: ${boxShadow};` : 'box-shadow: 30 3px 6px rgba(25, 25, 217, 0.2)'} |
There was a problem hiding this comment.
Fix typo in default box-shadow value.
The default box-shadow has a typo: 30 3px 6px should be 0 3px 6px. The first value is the horizontal offset and should be 0, not 30.
Apply this diff:
- ${boxShadow ? `box-shadow: ${boxShadow};` : 'box-shadow: 30 3px 6px rgba(25, 25, 217, 0.2)'}
+ ${boxShadow ? `box-shadow: ${boxShadow};` : 'box-shadow: 0 3px 6px rgba(25, 25, 217, 0.2);'}Note: Also added missing semicolon at the end of the default box-shadow value.
📝 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.
| ${boxShadow ? `box-shadow: ${boxShadow};` : 'box-shadow: 30 3px 6px rgba(25, 25, 217, 0.2)'} | |
| ${boxShadow ? `box-shadow: ${boxShadow};` : 'box-shadow: 0 3px 6px rgba(25, 25, 217, 0.2);'} |
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/reactTable/styles/styles.ts around line 169,
the default box-shadow string has a typo and missing semicolon: change the
default from "30 3px 6px rgba(25, 25, 217, 0.2)" to "0 3px 6px rgba(25, 25, 217,
0.2);" so the horizontal offset is 0 and the declaration ends with a semicolon;
update the template literal accordingly (keep using the existing conditional
that uses boxShadow when provided).
| const clickTimeoutRef = useRef<NodeJS.Timeout | null>(null); | ||
| const isDoubleClickRef = useRef(false); | ||
|
|
||
| const handleRowClick = (): void => { | ||
| onClick(row); | ||
| if (onRowClick) { | ||
| onRowClick(); | ||
| // Clear any existing timeout | ||
| if (clickTimeoutRef.current) { | ||
| clearTimeout(clickTimeoutRef.current); | ||
| clickTimeoutRef.current = null; | ||
| } | ||
|
|
||
| // Use a timeout to detect if this is part of a double-click | ||
| clickTimeoutRef.current = setTimeout(() => { | ||
| // Only execute click logic if it wasn't part of a double-click | ||
| if (!isDoubleClickRef.current) { | ||
| onClick(row); | ||
| if (onRowClick) { | ||
| onRowClick(); | ||
| } | ||
| } | ||
| isDoubleClickRef.current = false; | ||
| clickTimeoutRef.current = null; | ||
| }, 250); // 250ms timeout to detect double-click | ||
| }; |
There was a problem hiding this comment.
Double-click flag never reset causes first single-click after a double-click to be ignored
When a double-click occurs, handleRowDoubleClick sets isDoubleClickRef.current = true and clears clickTimeoutRef. Because the pending timeout never fires, isDoubleClickRef.current is not reset to false until some later click’s timeout callback runs. That means the first single-click after any double-click schedules a timeout whose callback sees isDoubleClickRef.current === true and skips onClick / onRowClick, effectively dropping that click.
You can fix this by resetting the flag at the start of handleRowClick so a new click always starts from a clean state:
const handleRowClick = (): void => {
- // Clear any existing timeout
- if (clickTimeoutRef.current) {
- clearTimeout(clickTimeoutRef.current);
- clickTimeoutRef.current = null;
- }
+ // Reset double-click flag for a fresh click sequence
+ isDoubleClickRef.current = false;
+
+ // Clear any existing timeout
+ if (clickTimeoutRef.current) {
+ clearTimeout(clickTimeoutRef.current);
+ clickTimeoutRef.current = null;
+ }Optionally, you could also drop isDoubleClickRef entirely and rely solely on clearing the pending timeout in handleRowDoubleClick, but the above is the minimal targeted fix.
Also applies to: 107-119, 121-128
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/reactTable/tableRow.tsx around lines 83-105
(and also apply same change to 107-119 and 121-128), handleRowClick must reset
the double-click flag at the start so a prior double-click doesn't cause the
very next single-click to be ignored; modify handleRowClick to set
isDoubleClickRef.current = false immediately when invoked (before scheduling a
timeout) so each click begins with a clean state, and ensure this change is
mirrored in the other two click-handling blocks mentioned.
shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
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)
168-169: Missing semicolon afterbackground-colormay break CSS parsing.The
background-colorvalue on line 168 is missing a trailing semicolon, which could cause the subsequentbox-shadowdeclaration to be ignored or malformed.Apply this diff:
- background-color: ${backgroundColor} - ${boxShadow ? `box-shadow: ${boxShadow};` : ''} + background-color: ${backgroundColor}; + ${boxShadow ? `box-shadow: ${boxShadow};` : ''}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
shesha-reactjs/src/components/autocomplete/index.tsx(3 hunks)shesha-reactjs/src/components/reactTable/styles/styles.ts(3 hunks)shesha-reactjs/src/components/reactTable/tableRow.tsx(3 hunks)shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts(1 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/reactTable/tableRow.tsxshesha-reactjs/src/components/autocomplete/index.tsxshesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.tsshesha-reactjs/src/components/reactTable/styles/styles.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: HackGenesis
Repo: shesha-io/shesha-framework PR: 4094
File: shesha-reactjs/src/components/dataTable/index.tsx:971-977
Timestamp: 2025-10-30T13:25:02.065Z
Learning: In the shesha-framework DataTable/ReactTable components, the useMultiSelect prop controls whether the checkbox selection column is added to the table, not the selection behavior itself. The actual selection behavior (single vs multiple) is controlled by the selectionMode prop. For both 'single' and 'multiple' modes, useMultiSelect should be true to show checkboxes. The ReactTable component's onSingleRowToggle function (around line 314-316 in shesha-reactjs/src/components/reactTable/index.tsx) enforces single-selection by clearing all other selections when a new row is selected in single mode.
📚 Learning: 2025-10-30T13:25:02.065Z
Learnt from: HackGenesis
Repo: shesha-io/shesha-framework PR: 4094
File: shesha-reactjs/src/components/dataTable/index.tsx:971-977
Timestamp: 2025-10-30T13:25:02.065Z
Learning: In the shesha-framework DataTable/ReactTable components, the useMultiSelect prop controls whether the checkbox selection column is added to the table, not the selection behavior itself. The actual selection behavior (single vs multiple) is controlled by the selectionMode prop. For both 'single' and 'multiple' modes, useMultiSelect should be true to show checkboxes. The ReactTable component's onSingleRowToggle function (around line 314-316 in shesha-reactjs/src/components/reactTable/index.tsx) enforces single-selection by clearing all other selections when a new row is selected in single mode.
Applied to files:
shesha-reactjs/src/components/reactTable/tableRow.tsxshesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.tsshesha-reactjs/src/components/reactTable/styles/styles.ts
📚 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/reactTable/tableRow.tsxshesha-reactjs/src/components/reactTable/styles/styles.ts
📚 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/reactTable/tableRow.tsxshesha-reactjs/src/components/autocomplete/index.tsxshesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.tsshesha-reactjs/src/components/reactTable/styles/styles.ts
📚 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/autocomplete/index.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/autocomplete/index.tsxshesha-reactjs/src/components/reactTable/styles/styles.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/designer-components/dataTable/table/migrations/migrate-v13.tsshesha-reactjs/src/components/reactTable/styles/styles.ts
📚 Learning: 2025-11-20T14:51:46.128Z
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.128Z
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/designer-components/dataTable/table/migrations/migrate-v13.tsshesha-reactjs/src/components/reactTable/styles/styles.ts
📚 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/reactTable/styles/styles.ts
📚 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/reactTable/styles/styles.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 (6)
shesha-reactjs/src/components/reactTable/tableRow.tsx (2)
86-131: Click/double-click detection logic looks correct.The implementation properly:
- Resets the double-click flag at the start of each click (line 88)
- Uses a 250ms debounce to distinguish single from double clicks
- Clears pending timeouts on double-click
- Cleans up on unmount
Minor observation: Line 105 (
isDoubleClickRef.current = false) is now redundant since the flag is reset at the start ofhandleRowClick(line 88), but it's harmless.
165-170: Unable to verify striped row logic—codebase files not accessible in sandbox.The file
shesha-reactjs/src/components/reactTable/tableRow.tsxand related styles could not be located despite multiple search attempts. The repository state in the sandbox environment does not contain the code under review.Please manually verify the striped row logic at lines 165–170:
- Confirm whether
index % 2 === 0applying thetrOddclass to even-indexed rows is intentional (0-based indexing where visual row 1 = index 0)- Check the CSS definition of
trOddto understand its intended visual styling- Ensure consistency between the class name and the index condition
shesha-reactjs/src/components/autocomplete/index.tsx (3)
78-101: Loading completion effect is well-structured but has redundant dependencies.The effect correctly handles loading completion and fallback scenarios. However:
keysis derived fromprops.value(line 64-71), so including both in the dependency array is slightly redundant. Consider using onlyprops.valueor extracting the key computation.The effect relies on
allDataandkeyValueFunc/outcomeValueFuncwhich are defined outside but not in deps. Since these are stable (defined at component level), this is acceptable, but ensure they don't change between renders.
137-158: Good defensive error handling around data source operations.The try/catch block properly handles cases where
setPredefinedFiltersmight throw (e.g., source not fully initialized). The fallback to existing values maintains a consistent user experience.
302-303: Simplified loading condition is correct.The removal of
keys.lengthgating is safe becauseloadingValuesis only set totruewhen there are keys that need resolution (line 133 checkskeys.lengthimplicitly viaallExist).shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts (1)
4-33: Unable to verify code changes due to inaccessible repository.The sandbox environment does not contain the source files necessary to verify whether
ITableComponentPropsincludes theuseMultiselectproperty for backward compatibility. The repository appears empty except for Git metadata and build configuration.The original review noted the migration logic is "now correct," and the code semantics align with the component behavior (confirmed by retrieved learning). However, I cannot definitively confirm:
- Whether
ITableComponentPropsincludesuseMultiselect?: booleanas a legacy property- Whether accessing
props.useMultiselecton line 9 will cause TypeScript errors- Whether the migration handles all legacy configurations correctly
Please manually verify that
ITableComponentPropsincludes theuseMultiselectproperty definition to ensure the migration safely accesses this property without type errors.
There was a problem hiding this comment.
Actionable comments posted: 2
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/autocomplete/index.tsx (1)
73-76: Inconsistent null-safety forsourcecauses potential crash.Line 76 accesses
source.errorwithout optional chaining, but lines 114-123 defensively handle!source. Ifsourceis everundefinedornull, line 76 will throw before reaching the new defensive check.Apply this diff to fix the inconsistency:
// reset loading state on error useEffect(() => { setLoadingValues(false); - }, [source.error]); + }, [source?.error]);Also applies to: 112-123
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
shesha-reactjs/src/components/autocomplete/index.tsx(3 hunks)shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts(1 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/autocomplete/index.tsxshesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: HackGenesis
Repo: shesha-io/shesha-framework PR: 4094
File: shesha-reactjs/src/components/dataTable/index.tsx:971-977
Timestamp: 2025-10-30T13:25:02.065Z
Learning: In the shesha-framework DataTable/ReactTable components, the useMultiSelect prop controls whether the checkbox selection column is added to the table, not the selection behavior itself. The actual selection behavior (single vs multiple) is controlled by the selectionMode prop. For both 'single' and 'multiple' modes, useMultiSelect should be true to show checkboxes. The ReactTable component's onSingleRowToggle function (around line 314-316 in shesha-reactjs/src/components/reactTable/index.tsx) enforces single-selection by clearing all other selections when a new row is selected in single mode.
📚 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/autocomplete/index.tsxshesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts
📚 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/autocomplete/index.tsx
📚 Learning: 2025-10-30T13:25:02.065Z
Learnt from: HackGenesis
Repo: shesha-io/shesha-framework PR: 4094
File: shesha-reactjs/src/components/dataTable/index.tsx:971-977
Timestamp: 2025-10-30T13:25:02.065Z
Learning: In the shesha-framework DataTable/ReactTable components, the useMultiSelect prop controls whether the checkbox selection column is added to the table, not the selection behavior itself. The actual selection behavior (single vs multiple) is controlled by the selectionMode prop. For both 'single' and 'multiple' modes, useMultiSelect should be true to show checkboxes. The ReactTable component's onSingleRowToggle function (around line 314-316 in shesha-reactjs/src/components/reactTable/index.tsx) enforces single-selection by clearing all other selections when a new row is selected in single mode.
Applied to files:
shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.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/designer-components/dataTable/table/migrations/migrate-v13.ts
📚 Learning: 2025-11-20T14:51:46.128Z
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.128Z
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/designer-components/dataTable/table/migrations/migrate-v13.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 (5)
shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts (3)
1-3: LGTM!Import statements are appropriate for a migration file.
8-25: Verify handling of non-boolean useMultiselect values.The migration correctly handles boolean
useMultiselectvalues (converting and removing them), but whenuseMultiselectexists with a non-boolean value (e.g., explicitly set toundefined,null, or an invalid type), it falls through to the fallback case at line 29, which preserves the property.Should the migration remove
useMultiselectregardless of its type, or is preserving non-boolean values intentional?If all
useMultiselectproperties should be removed during migration, apply this diff:if (hasUseMultiselect) { const useMultiselect = props.useMultiselect; - // If useMultiselect was explicitly set + // Remove useMultiselect property regardless of type + const { useMultiselect: removed, ...propsWithoutUseMultiselect } = props; + + // If useMultiselect was explicitly set to a boolean, convert it if (typeof useMultiselect === 'boolean') { - // If useMultiselect was true, set selectionMode to 'multiple' - // If useMultiselect was false, set selectionMode to 'none' (no checkboxes) const newSelectionMode = useMultiselect ? 'multiple' : 'none'; - - // Remove the old useMultiselect property and set the new selectionMode - const { useMultiselect: removed, ...propsWithoutUseMultiselect } = props; - return { ...propsWithoutUseMultiselect, selectionMode: newSelectionMode, }; } + + // For non-boolean values, just remove useMultiselect and ensure default selectionMode + return { + ...propsWithoutUseMultiselect, + selectionMode: propsWithoutUseMultiselect.selectionMode || 'none', + }; }Based on learnings, the migration correctly maps boolean values to their new equivalents.
27-32: LGTM!The fallback logic correctly ensures a default
selectionModewhenuseMultiselectis absent. The use of||operator appropriately defaults falsy values to'none'.shesha-reactjs/src/components/autocomplete/index.tsx (2)
166-166: LGTM!Adding
source?.isInProgress?.fetchTableDatato the dependency array ensures the effect reacts appropriately when the data source's fetch state changes.
298-299: LGTM!Simplifying the loading condition to rely solely on
loadingIndicatorandloadingValues(withreadOnlyguard) is cleaner and more semantically accurate than the previous keys-length gating.
There was a problem hiding this comment.
Actionable comments posted: 1
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/autocomplete/index.tsx (1)
79-81: Add optional chaining to prevent potential runtime error.
sourcefromuseDataTableStore(false)can be undefined, butsource.erroris accessed without optional chaining.useEffect(() => { setLoadingValues(false); - }, [source.error]); + }, [source?.error]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/components/autocomplete/index.tsx(5 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/autocomplete/index.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: HackGenesis
Repo: shesha-io/shesha-framework PR: 4094
File: shesha-reactjs/src/components/dataTable/index.tsx:971-977
Timestamp: 2025-10-30T13:25:02.065Z
Learning: In the shesha-framework DataTable/ReactTable components, the useMultiSelect prop controls whether the checkbox selection column is added to the table, not the selection behavior itself. The actual selection behavior (single vs multiple) is controlled by the selectionMode prop. For both 'single' and 'multiple' modes, useMultiSelect should be true to show checkboxes. The ReactTable component's onSingleRowToggle function (around line 314-316 in shesha-reactjs/src/components/reactTable/index.tsx) enforces single-selection by clearing all other selections when a new row is selected in single mode.
📚 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/autocomplete/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/autocomplete/index.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/autocomplete/index.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/autocomplete/index.tsx
📚 Learning: 2025-11-25T07:17:44.436Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.436Z
Learning: Applies to **/*{Component,Editor}.{ts,tsx} : Do not use `useEffect` for resetting values to defaults in controlled editor components; this causes unnecessary re-renders
Applied to files:
shesha-reactjs/src/components/autocomplete/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-attempt
🔇 Additional comments (4)
shesha-reactjs/src/components/autocomplete/index.tsx (4)
21-25: LGTM! Good extraction of repeated normalization logic.This helper correctly uses
unknowntype per coding guidelines and consolidates the duplicated fallback pattern.
83-104: LGTM! Loading completion effect properly handles data resolution.The effect correctly:
- Includes memoized functions in dependencies (addressing previous stale closure concerns)
- Handles both successful data resolution and fallback scenarios
- Uses the extracted
getNormalizedValueshelper consistently
115-167: LGTM! Good defensive error handling and consistent normalization.The changes properly:
- Add guards for missing data source with appropriate fallbacks
- Include error logging in the catch block (addressing previous concern about silent error swallowing)
- Use
getNormalizedValuesconsistently throughout- Update dependencies to reflect data source readiness state
299-300: LGTM!Clean loading state derivation that properly accounts for read-only mode.
There was a problem hiding this comment.
Actionable comments posted: 2
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/autocomplete/index.tsx (1)
79-81: Potential null reference in dependency array.The effect at line 79-81 uses
source.errorin the dependency array, butsourcecan beundefined(fromuseDataTableStore(false)on line 32). This could cause issues:useEffect(() => { setLoadingValues(false); - }, [source.error]); + }, [source?.error]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/components/autocomplete/index.tsx(8 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/autocomplete/index.tsx
🧠 Learnings (7)
📓 Common learnings
Learnt from: HackGenesis
Repo: shesha-io/shesha-framework PR: 4094
File: shesha-reactjs/src/components/dataTable/index.tsx:971-977
Timestamp: 2025-10-30T13:25:02.065Z
Learning: In the shesha-framework DataTable/ReactTable components, the useMultiSelect prop controls whether the checkbox selection column is added to the table, not the selection behavior itself. The actual selection behavior (single vs multiple) is controlled by the selectionMode prop. For both 'single' and 'multiple' modes, useMultiSelect should be true to show checkboxes. The ReactTable component's onSingleRowToggle function (around line 314-316 in shesha-reactjs/src/components/reactTable/index.tsx) enforces single-selection by clearing all other selections when a new row is selected in single mode.
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.
📚 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/autocomplete/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/autocomplete/index.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/autocomplete/index.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/autocomplete/index.tsx
📚 Learning: 2025-11-25T07:17:44.436Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.436Z
Learning: Applies to **/*{Component,Editor}.{ts,tsx} : Do not use `useEffect` for resetting values to defaults in controlled editor components; this causes unnecessary re-renders
Applied to files:
shesha-reactjs/src/components/autocomplete/index.tsx
📚 Learning: 2025-11-25T07:17:44.436Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.436Z
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/autocomplete/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-attempt
🔇 Additional comments (4)
shesha-reactjs/src/components/autocomplete/index.tsx (4)
21-25: Good extraction of the normalization helper.This consolidates the previously duplicated normalization logic into a reusable function with proper
unknowntyping per coding guidelines.
115-167: Good defensive handling for missing data source.The added guards for when
sourceis unavailable (lines 117-125) and proper error handling with logging (line 140) address previous review concerns. The fallback togetNormalizedValuesprovides consistent behavior.
205-230: Good type improvements in event handlers.The
unknowntype for_valueandoptionparameters with appropriate type assertions provides better type safety while maintaining runtime behavior.
299-309: LGTM!The loading indicator logic properly combines both loading states and respects the
readOnlyprop to avoid showing spinners inappropriately.
2 participants
#2603
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.