Skip to content

Derik update/bug/2603#4283

Merged
James-Baloyi merged 11 commits intoshesha-io:mainfrom
HackGenesis:derik-update/bug/2603
Dec 3, 2025
Merged

Derik update/bug/2603#4283
James-Baloyi merged 11 commits intoshesha-io:mainfrom
HackGenesis:derik-update/bug/2603

Conversation

@HackGenesis
Copy link
Copy Markdown
Contributor

@HackGenesis HackGenesis commented Nov 30, 2025

#2603

Summary by CodeRabbit

  • New Features

    • Programmatic row toggle exposed; button actions can receive an optional dynamic item; new table styling settings and defaults (header and body controls).
  • Bug Fixes

    • Checkboxes only show in multi-select; row clicks now select in single and multi modes; multi-select activation logic tightened; striped styling forwarded correctly.
  • Refactor

    • Default selection mode moved from none to single; appearance settings simplified and migrations added.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Nov 30, 2025

Walkthrough

Selection defaults and selection handling were tightened and unified: multi-select requires explicit 'multiple'; row clicks now trigger selection for 'single' and 'multiple'; React table exposes toggleRowSelected; designer table gained styling defaults, new migrations, settings UI changes, and dynamic action execution with typed dynamicItem.

Changes

Cohort / File(s) Summary
Selection UI & Click Handling
shesha-reactjs/src/components/dataList/index.tsx, shesha-reactjs/src/components/dataTable/index.tsx
Checkbox rendered only when selectionMode === 'multiple'; row clicks invoke selection for single and multiple; useMultiSelect calculation narrowed to exclude single.
React Table: selection API & row state
shesha-reactjs/src/components/reactTable/index.tsx, shesha-reactjs/src/components/reactTable/tableRow.tsx
toggleRowSelected exposed/used; handleSelectRow clears single selection, toggles via toggleRowSelected(row.id), then calls onSelectRow; row class now also checks per-row isSelected.
Designer: migrations & selection defaults
shesha-reactjs/src/designer-components/_legacyComponents/listControl/index.tsx, shesha-reactjs/src/designer-components/dataList/dataListComponent.tsx, shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts
Default selectionMode changed from 'none''single' in migrators/components; migrate-v13 now maps useMultiselect: false'single'.
Designer: table styling migrations & defaults
shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v16.ts, shesha-reactjs/src/designer-components/dataTable/table/migrate-v15toV16..., shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx
Added migrateV15toV16 to apply styling defaults; table init merges getTableDefaults()/getTableSettingsDefaults(); migrator sequence extended and new styling props initialized (rowHeight, rowPadding, rowBorder, headerFontSize, headerFontWeight).
Designer: table defaults & helpers
shesha-reactjs/src/designer-components/dataTable/table/models.ts, shesha-reactjs/src/designer-components/dataTable/table/utils.ts
Added optional tableSettings shape to component props; added getTableDefaults() and getTableSettingsDefaults() helpers returning default styling values.
Designer: appearance UI refactor
shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts
Replaced previous nested appearance config with separate "Header Styling" and "Table Body Styling" panels and explicit inputs for header/body styling (font size/weight, colors, row height/padding/border).
Prop forwarding
shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx
striped prop forwarded to internal components/DataTable invocation.
Dynamic action context & types
shesha-reactjs/src/designer-components/profileDropdown/index.tsx, shesha-reactjs/src/designer-components/profileDropdown/utils.tsx, shesha-reactjs/src/providers/buttonGroupConfigurator/models.ts, shesha-reactjs/src/designer-components/button/configurableButton/index.tsx
Replaced executeActionViaConfiguration with dispatcher executeAction; added executeActionWithDynamicContext that injects available constants and optional typed dynamicItem; getMenuItem accepts optional dynamicItem; IButtonItem.dynamicItem and configurableButton dynamicItem typed as IFullAuditedEntity.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay special attention to migration impact on persisted models (migrate-v13 and migrateV15toV16) and defaulting logic.
  • Verify selection interactions across DataList, DataTable, ReactTable (consistency of selectionMode, useMultiSelect, toggleRowSelected, event ordering).
  • Review tableSettings UI bindings and that new default values propagate into rendered components and saved models.

Possibly related PRs

  • Derik/en/2603 #4094 — overlaps selectionMode/useMultiSelect and table selection behavior changes.
  • expose dynamic items #4122 — related changes to profileDropdown, configurableButton, and dynamicItem/action execution wiring.
  • refactor events #3978 — directly touches DataList row-click selection handling and may conflict with the row-click selection behavior here.

Suggested reviewers

  • James-Baloyi
  • IvanIlyichev

Poem

🐰 I hopped through rows and toggled light,

Single clicks and multiples take flight,
I tuck context into every run,
Defaults settled, styles under the sun,
A little rabbit's code-made delight.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Derik update/bug/2603' is vague and does not clearly convey the actual changes made in the pull request, which involve selection mode updates, row selection handling, table styling improvements, and button configuration enhancements across multiple components. Provide a more descriptive title that summarizes the main changes, such as 'Update selection modes and add table styling configuration' or similar that better reflects the changeset content.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b4c062 and febee80.

📒 Files selected for processing (13)
  • shesha-reactjs/src/components/dataList/index.tsx (2 hunks)
  • shesha-reactjs/src/components/dataTable/index.tsx (1 hunks)
  • shesha-reactjs/src/components/reactTable/index.tsx (2 hunks)
  • shesha-reactjs/src/components/reactTable/tableRow.tsx (1 hunks)
  • shesha-reactjs/src/designer-components/_legacyComponents/listControl/index.tsx (1 hunks)
  • shesha-reactjs/src/designer-components/button/configurableButton/index.tsx (1 hunks)
  • shesha-reactjs/src/designer-components/dataList/dataListComponent.tsx (1 hunks)
  • shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts (2 hunks)
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx (2 hunks)
  • shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx (1 hunks)
  • shesha-reactjs/src/designer-components/profileDropdown/index.tsx (3 hunks)
  • shesha-reactjs/src/designer-components/profileDropdown/utils.tsx (4 hunks)
  • shesha-reactjs/src/providers/buttonGroupConfigurator/models.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type 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/designer-components/button/configurableButton/index.tsx
  • shesha-reactjs/src/components/reactTable/tableRow.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx
  • shesha-reactjs/src/designer-components/dataList/dataListComponent.tsx
  • shesha-reactjs/src/providers/buttonGroupConfigurator/models.ts
  • shesha-reactjs/src/designer-components/profileDropdown/index.tsx
  • shesha-reactjs/src/designer-components/profileDropdown/utils.tsx
  • shesha-reactjs/src/components/reactTable/index.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts
  • shesha-reactjs/src/components/dataList/index.tsx
  • shesha-reactjs/src/designer-components/_legacyComponents/listControl/index.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx
  • shesha-reactjs/src/components/dataTable/index.tsx
**/*{Component,Editor}.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*{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
Remove the defaultValue pattern entirely from controlled editor components
Do not use useEffect for resetting values to defaults in controlled editor components; this causes unnecessary re-renders

Files:

  • shesha-reactjs/src/designer-components/dataList/dataListComponent.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx
🧠 Learnings (14)
📓 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-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/designer-components/button/configurableButton/index.tsx
  • shesha-reactjs/src/designer-components/dataList/dataListComponent.tsx
  • shesha-reactjs/src/providers/buttonGroupConfigurator/models.ts
  • shesha-reactjs/src/designer-components/profileDropdown/index.tsx
  • shesha-reactjs/src/designer-components/profileDropdown/utils.tsx
  • shesha-reactjs/src/components/dataList/index.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx
  • shesha-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/button/configurableButton/index.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx
  • shesha-reactjs/src/designer-components/dataList/dataListComponent.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts
  • shesha-reactjs/src/designer-components/_legacyComponents/listControl/index.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.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/components/reactTable/tableRow.tsx
  • shesha-reactjs/src/designer-components/dataList/dataListComponent.tsx
  • shesha-reactjs/src/components/reactTable/index.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts
  • shesha-reactjs/src/components/dataList/index.tsx
  • shesha-reactjs/src/designer-components/_legacyComponents/listControl/index.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx
  • shesha-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/tableWrapper.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.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/designer-components/dataTable/table/tableWrapper.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/tableWrapper.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/designer-components/profileDropdown/index.tsx
  • shesha-reactjs/src/designer-components/profileDropdown/utils.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/designer-components/profileDropdown/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/designer-components/profileDropdown/index.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/designer-components/profileDropdown/utils.tsx
  • shesha-reactjs/src/components/dataList/index.tsx
  • shesha-reactjs/src/designer-components/_legacyComponents/listControl/index.tsx
  • shesha-reactjs/src/components/dataTable/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} : Remove the `defaultValue` pattern entirely from controlled editor components

Applied to files:

  • shesha-reactjs/src/designer-components/_legacyComponents/listControl/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/designer-components/dataTable/table/tableComponent.tsx
📚 Learning: 2025-11-04T14:15:44.418Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4138
File: shesha-reactjs/src/components/calendar/index.tsx:91-93
Timestamp: 2025-11-04T14:15:44.418Z
Learning: In shesha-reactjs calendar components (shesha-reactjs/src/components/calendar/index.tsx), external date ranges for all-day events use an inclusive format where both start and end dates at midnight represent the actual date range to display. The calendar always converts these to react-big-calendar's exclusive format by adding 1 day to the end when both start and end are at start of day, regardless of whether the end is already after the start. This correctly handles both single-day selections (start = end at midnight) and multi-day ranges (start and end are different midnights).

Applied to files:

  • shesha-reactjs/src/components/dataTable/index.tsx
🔇 Additional comments (15)
shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx (1)

355-355: LGTM: Striped prop correctly forwarded to DataTable.

The striped prop is now properly passed through to the DataTable component, ensuring consistent styling behavior. This complements the existing usage at line 76 where the prop configures wrapper-level styles.

Note: Manual verification is required to confirm that the DataTable component's TypeScript definition accepts the striped prop to ensure no type errors are introduced.

shesha-reactjs/src/designer-components/button/configurableButton/index.tsx (1)

10-10: LGTM! Type safety improvement.

Replacing any with IFullAuditedEntity for the dynamicItem prop strengthens type safety and aligns with the coding guideline to eliminate any types.

As per coding guidelines: "Eliminate the any type; use unknown type instead for values with unknown types, forcing explicit type checking."

Also applies to: 16-16

shesha-reactjs/src/designer-components/_legacyComponents/listControl/index.tsx (1)

36-36: LGTM! Default selection mode updated.

The default selectionMode change from 'none' to 'single' is consistent with similar migrator updates across the PR (dataList, dataTable). This aligns the deprecated ListComponent with the new default selection behavior.

shesha-reactjs/src/designer-components/dataList/dataListComponent.tsx (1)

38-38: LGTM! Default selection mode updated.

The default selectionMode change from 'none' to 'single' in the migrator is consistent with the PR-wide shift toward single-selection defaults.

shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx (2)

65-65: LGTM! Default selection mode updated.

The default selectionMode change from 'none' to 'single' is consistent with the PR-wide shift toward single-selection defaults.


46-54: No action needed—current property precedence is intentional.

The { ...model, ...defaults } spread order is consistent with the established design pattern in this codebase's designer components (e.g., chart migrators), where defaults intentionally override existing properties to reset to new defaults. This is by design and should not be changed.

shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts (1)

29-32: LGTM! Default selection mode updated.

The default selectionMode fallback to 'single' is consistent with the PR-wide shift in selection defaults.

shesha-reactjs/src/components/reactTable/index.tsx (2)

287-287: LGTM! Exposing toggleRowSelected for programmatic selection control.

Exposing toggleRowSelected from the useTable hook enables the updated selection logic in handleSelectRow.


426-440: LGTM! Enhanced row selection logic.

The updated handleSelectRow properly handles both single and multiple selection modes:

  • For single selection: clears all selections before toggling the current row
  • For multiple selection: toggles the current row while preserving existing selections

The logic correctly respects omitClick and inline edit/delete modes.

Based on learnings: The ReactTable component's selection behavior is controlled by the selectionMode prop, with useMultiSelect controlling only whether the checkbox column is added.

shesha-reactjs/src/components/dataList/index.tsx (2)

605-605: LGTM! Checkbox visibility restricted to multiple selection mode.

Restricting checkbox visibility to selectionMode === 'multiple' (previously !== 'none') is more intuitive. Checkboxes don't make sense for single-selection scenarios where row clicks suffice.


623-632: LGTM! Extended row click handling to both selection modes.

Extending row click selection to both 'single' and 'multiple' modes makes the interaction more consistent. Users can click rows to select them regardless of the selection mode, with onSelectRowLocal handling the mode-specific logic.

shesha-reactjs/src/components/reactTable/tableRow.tsx (1)

169-169: LGTM! Per-row selection state integrated.

Adding || row?.isSelected to the selection condition allows rows to be marked as selected via both the external selectedRowIndex prop and the internal isSelected state from react-table's useRowSelect hook. This provides more flexible selection state management.

Based on learnings: This change integrates with the broader selection refactoring where toggleRowSelected is now exposed and used for programmatic row selection in reactTable/index.tsx.

shesha-reactjs/src/providers/buttonGroupConfigurator/models.ts (1)

7-7: Extending IButtonItem with dynamicItem looks consistent

The optional dynamicItem?: IFullAuditedEntity on IButtonItem cleanly extends the shape without breaking existing consumers and matches how dynamicItem is read through isButtonItem in the dropdown utilities. No issues from a typing or compatibility standpoint.

Also applies to: 69-72

shesha-reactjs/src/designer-components/profileDropdown/index.tsx (1)

2-3: Align executeActionWithDynamicContext signature with optional configurations

executeActionWithDynamicContext is declared with a non-optional actionConfiguration parameter, but it's called via getMenuItem with item.actionConfiguration, which is optional on IButtonItem. The existing defensive guard if (actionConfiguration) indicates this optionality should be reflected in the type signature.

Proposed change:

-    const executeActionWithDynamicContext = (actionConfiguration: IConfigurableActionConfiguration, dynamicItem?: IFullAuditedEntity) => {
+    const executeActionWithDynamicContext = (actionConfiguration?: IConfigurableActionConfiguration, dynamicItem?: IFullAuditedEntity) => {
       if (actionConfiguration) {
         executeAction({
           actionConfiguration,
           argumentsEvaluationContext: { ...allData, dynamicItem },
         });
       }
     };

This aligns the type system with both the runtime behavior and the optional nature of configurations passed from the caller.

shesha-reactjs/src/designer-components/profileDropdown/utils.tsx (1)

48-52: Guard against missing actionConfiguration when wiring menu item clicks

IButtonItem.actionConfiguration should be guarded at the call site in onClick to ensure type safety. The execute callback is typed to require IConfigurableActionConfiguration, but actionConfiguration may not always be present on button items.

Check both conditions before invoking execute:

-      onClick: () => isButtonItem(item) ? execute(item.actionConfiguration, dynamicItem) : undefined,
+      onClick: () => {
+        if (isButtonItem(item) && item.actionConfiguration) {
+          execute(item.actionConfiguration, dynamicItem);
+        }
+      },

This ensures execute is only called when both the item is a button and has a valid configuration, keeping types consistent with the callback signature.

Also applies to: 59-60, 69-70

Comment on lines 12 to +15
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';
const newSelectionMode = useMultiselect ? 'multiple' : 'single';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update the comment to reflect the new mapping.

The comment on line 14 states "set selectionMode to 'none'" but the code now maps false to 'single'. The comment should be updated to match the implementation.

Apply this diff:

     // If useMultiselect was explicitly set
     if (typeof useMultiselect === 'boolean') {
       // If useMultiselect was true, set selectionMode to 'multiple'
-      // If useMultiselect was false, set selectionMode to 'none' (no checkboxes)
+      // If useMultiselect was false, set selectionMode to 'single'
       const newSelectionMode = useMultiselect ? 'multiple' : 'single';
📝 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.

Suggested change
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';
const newSelectionMode = useMultiselect ? 'multiple' : 'single';
if (typeof useMultiselect === 'boolean') {
// If useMultiselect was true, set selectionMode to 'multiple'
// If useMultiselect was false, set selectionMode to 'single'
const newSelectionMode = useMultiselect ? 'multiple' : 'single';
🤖 Prompt for AI Agents
In
shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v13.ts
around lines 12 to 15, the inline comment incorrectly says "set selectionMode to
'none'" when the code maps useMultiselect === false to 'single'; update the
comment to reflect the actual mapping (i.e., when useMultiselect is true set
selectionMode to 'multiple', when false set selectionMode to 'single') so the
comment matches the implementation.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between febee80 and 5a9e06b.

📒 Files selected for processing (1)
  • shesha-reactjs/src/designer-components/profileDropdown/index.tsx (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type 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/designer-components/profileDropdown/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-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/designer-components/profileDropdown/index.tsx
📚 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/designer-components/profileDropdown/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/designer-components/profileDropdown/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/profileDropdown/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/designer-components/profileDropdown/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 (3)
shesha-reactjs/src/designer-components/profileDropdown/index.tsx (3)

1-15: New configurable-action and entity imports are consistent

The added imports (IConfigurableActionConfiguration, useConfigurableActionDispatcher, useAvailableConstantsData, IFullAuditedEntity) align with the new dynamic-context execution path and keep types explicit without introducing any. No issues here.


80-85: Hooking into dispatcher and constants data is appropriate

Using useConfigurableActionDispatcher to obtain executeAction and useAvailableConstantsData for the evaluation context fits the configurable-actions pattern and keeps the ProfileDropdown aligned with other configurable components that execute actions against a shared context.


155-155: Menu item wiring now correctly passes dynamic execution context

Passing executeActionWithDynamicContext into getMenuItem ensures menu items can execute configurable actions with both the shared constants data and any per-item dynamicItem. This matches the dynamic-item support introduced elsewhere and keeps the execution path centralized in one helper.

Comment on lines +145 to +153
// Custom execute function that includes dynamicItem in the context
const executeActionWithDynamicContext = (actionConfiguration: IConfigurableActionConfiguration, dynamicItem?: IFullAuditedEntity): void => {
if (actionConfiguration) {
executeAction({
actionConfiguration,
argumentsEvaluationContext: { ...allData, dynamicItem },
});
}
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Dynamic context wrapper is correct; consider stabilizing with useCallback

The executeActionWithDynamicContext helper correctly:

  • Guards against missing actionConfiguration.
  • Injects both allData and an optional dynamicItem into argumentsEvaluationContext, with dynamicItem cleanly layered on top.

To avoid unnecessary re-renders in consumers that might depend on function identity, you could optionally wrap this helper in useCallback:

-    const executeActionWithDynamicContext = (actionConfiguration: IConfigurableActionConfiguration, dynamicItem?: IFullAuditedEntity): void => {
-      if (actionConfiguration) {
-        executeAction({
-          actionConfiguration,
-          argumentsEvaluationContext: { ...allData, dynamicItem },
-        });
-      }
-    };
+    const executeActionWithDynamicContext = React.useCallback(
+      (actionConfiguration: IConfigurableActionConfiguration, dynamicItem?: IFullAuditedEntity): void => {
+        if (!actionConfiguration) return;
+        executeAction({
+          actionConfiguration,
+          argumentsEvaluationContext: { ...allData, dynamicItem },
+        });
+      },
+      [executeAction, allData],
+    );

Not mandatory unless downstream memoization depends on a stable callback.

🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/profileDropdown/index.tsx around lines
145 to 153, the helper executeActionWithDynamicContext should be stabilized with
useCallback to avoid changing function identity on each render; wrap the
function in React.useCallback, include executeAction and allData (and any other
values referenced) in the dependency array, and keep the same parameter
signature and guard logic so behavior is unchanged.

@HackGenesis HackGenesis marked this pull request as ready for review December 2, 2025 07:39
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a9e06b and 2a312f3.

📒 Files selected for processing (5)
  • shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v16.ts (1 hunks)
  • shesha-reactjs/src/designer-components/dataTable/table/models.ts (1 hunks)
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx (5 hunks)
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts (2 hunks)
  • shesha-reactjs/src/designer-components/dataTable/table/utils.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type 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/designer-components/dataTable/table/utils.ts
  • shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v16.ts
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts
  • shesha-reactjs/src/designer-components/dataTable/table/models.ts
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx
**/*{Component,Editor}.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*{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
Remove the defaultValue pattern entirely from controlled editor components
Do not use useEffect for resetting values to defaults in controlled editor components; this causes unnecessary re-renders

Files:

  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx
🧠 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-12-02T14:32:10.802Z
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.802Z
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/dataTable/table/utils.ts
  • shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v16.ts
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts
  • shesha-reactjs/src/designer-components/dataTable/table/models.ts
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.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/utils.ts
  • shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v16.ts
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts
  • shesha-reactjs/src/designer-components/dataTable/table/models.ts
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.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/utils.ts
  • shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v16.ts
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts
  • shesha-reactjs/src/designer-components/dataTable/table/models.ts
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.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/tableSettings.ts
  • shesha-reactjs/src/designer-components/dataTable/table/models.ts
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.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.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/designer-components/dataTable/table/tableSettings.ts
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx
📚 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/designer-components/dataTable/table/models.ts
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.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/tableComponent.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/designer-components/dataTable/table/tableComponent.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 (5)
shesha-reactjs/src/designer-components/dataTable/table/utils.ts (1)

62-94: LGTM!

The utility functions are well-structured with explicit return types. getTableSettingsDefaults properly reuses getTableDefaults to avoid value duplication. The default CSS values are sensible.

shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts (2)

511-586: LGTM!

The new Header Styling and Table Body Styling collapsible panels are well-organized with appropriate tooltips and consistent structure. The settings form components follow the established patterns in this file.


1082-1108: LGTM!

The simplified color picker inputs for header and sort indicator colors improve accessibility of these settings. The approach is consistent with other styling inputs in the form.

shesha-reactjs/src/designer-components/dataTable/table/models.ts (1)

60-68: Clarify the relationship between flat styling properties and nested tableSettings.

The tableSettings property appears to duplicate existing flat properties (rowHeight, rowPadding, rowBorder, headerFontSize, headerFontWeight). This creates potential ambiguity about which values take precedence and could lead to sync issues.

Consider documenting the intended usage or consolidating to a single source of truth. If both are intentionally maintained for backward compatibility or form binding, add a comment explaining the relationship and precedence rules.

shesha-reactjs/src/designer-components/dataTable/table/migrations/migrate-v16.ts (1)

5-16: Verify if tableSettings nested structure requires population in this migration.

The migration currently initializes only flat properties using nullish coalescing. Confirm whether tableSettings should also be populated here or if it's initialized elsewhere during component instantiation. Check other migrations in this directory to understand the established pattern for nested structure handling.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx (2)

131-140: Duplicate default assignments in migration steps 16 and 17.

This concern was already flagged in a previous review. If migrateV15toV16 sets the same defaults (rowHeight, rowPadding, rowBorder, headerFontSize, headerFontWeight), step 17 is redundant. Consolidate into a single migration step.


141-150: Logic allows all properties through; allowedStyleProperties check is ineffective.

This was flagged in a previous review. The boolean (name !== 'items' || isPropertySettings(value)) || allowedStyleProperties.includes(name) short-circuits to true for any name !== 'items', making allowedStyleProperties checking dead code for non-'items' properties. Fix per the previous suggestion or clarify intent with an updated comment.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2a312f3 and 9ca9d69.

📒 Files selected for processing (2)
  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx (5 hunks)
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type 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/designer-components/dataTable/table/tableComponent.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts
**/*{Component,Editor}.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*{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
Remove the defaultValue pattern entirely from controlled editor components
Do not use useEffect for resetting values to defaults in controlled editor components; this causes unnecessary re-renders

Files:

  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx
🧠 Learnings (13)
📓 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: 4280
File: shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts:205-227
Timestamp: 2025-12-02T14:32:10.802Z
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.
📚 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/tableComponent.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts
📚 Learning: 2025-12-02T14:32:10.802Z
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.802Z
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/dataTable/table/tableComponent.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.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/designer-components/dataTable/table/tableComponent.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts
📚 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/tableComponent.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/tableComponent.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.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/designer-components/dataTable/table/tableComponent.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/designer-components/dataTable/table/tableComponent.tsx
📚 Learning: 2025-11-03T13:34:50.731Z
Learnt from: James-Baloyi
Repo: shesha-io/shesha-framework PR: 4124
File: shesha-reactjs/src/designer-components/profileDropdown/index.tsx:109-142
Timestamp: 2025-11-03T13:34:50.731Z
Learning: In the shesha-framework codebase, the `anyOfPermissionsGranted` function in `shesha-reactjs/src/providers/sheshaApplication/application.ts` returns `true` when passed an empty permissions array (`permissions?.length === 0`). This means items without permissions remain visible by default.

Applied to files:

  • shesha-reactjs/src/designer-components/dataTable/table/tableComponent.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/tableComponent.tsx
  • shesha-reactjs/src/designer-components/dataTable/table/tableSettings.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 **/*{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/designer-components/dataTable/table/tableComponent.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/designer-components/dataTable/table/tableSettings.ts
📚 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/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 (8)
shesha-reactjs/src/designer-components/dataTable/table/tableComponent.tsx (2)

47-60: Proper defaults initialization pattern.

The initModel correctly spreads defaults before the model, ensuring user-provided values override defaults. This aligns with the controlled component pattern.


71-71: Verify intentional default change from 'none' to 'single'.

The default selectionMode changed from 'none' to 'single'. Existing tables without an explicit selectionMode will now enable single-row selection after migration. Confirm this breaking change aligns with the intended fix for issue #2603.

shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts (6)

511-545: Header Styling panel structure looks good.

The collapsible panel configuration is clean with appropriate input types. jsSetting: false is correctly set for static styling values.


546-586: Table Body Styling panel is well-structured.

Input configurations are consistent with good tooltips explaining expected CSS value formats.


587-601: Property Router correctly handles device-responsive styling.

The propertyRouteName code expression properly reads designerDevice from canvas context with 'desktop' fallback.


809-901: Background type-based visibility conditions are consistent.

The hidden conditions correctly gate inputs based on the selected background type per device context.


995-1081: Table Specific panel provides comprehensive row styling options.

The styling options (striped, hover, color states, sticky header) with jsSetting: true enable dynamic configuration which is appropriate for these runtime-adjustable properties.


1109-1147: Custom Styles panel provides appropriate escape hatches.

The code editors for style, tableStyle, and containerStyle with clear descriptions allow advanced customization beyond the structured inputs.

@James-Baloyi James-Baloyi merged commit 9f4e116 into shesha-io:main Dec 3, 2025
2 checks passed
This was referenced Dec 5, 2025
@coderabbitai coderabbitai bot mentioned this pull request Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants