Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes consolidate and expand the handling of double-click events across the data table components. The double-click property is removed from one interface and added to another, allowing it to accept either a configurable action or a callback function. In the ReactTable component, the double-click functionality is enhanced through a new method that dispatches actions using a configurable action dispatcher. Additionally, designer components have been updated with new properties and extended settings for appearance, security, and events, with the TableWrapper now passing the new double-click configuration to the DataTable component. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant RT as ReactTable
participant Dispatcher as ConfigurableActionDispatcher
participant Action as ConfigurableAction
User->>RT: Double-click on row (rowData, index)
RT->>RT: Call performOnRowDoubleClick(rowData, index)
RT->>Dispatcher: Dispatch onRowDoubleClick action
Dispatcher->>Action: Execute action or callback
Action-->>Dispatcher: Action result
Dispatcher-->>RT: Return result
sequenceDiagram
participant TW as TableWrapper
participant DT as DataTable
TW->>DT: Pass onDblClick (from dblClickActionConfiguration)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
shesha-reactjs/src/components/reactTable/index.tsx (3)
328-345: Ensure consistent usage of row data.
Currently,performOnRowDoubleClickaccepts the entire row but refers to it asdata. If only the row's original data is needed, consider passingrow.originalinstead, to avoid confusion or unintended property usage.-return (data,) => { - const evaluationContext = { - data, - }; +return (row) => { + const evaluationContext = { + data: row.original, + };
347-353: ValidateonRowDoubleClicktype.
The code assumes anobjectis anIConfigurableActionConfigurationand a function implies a callback. Consider adding a safeguard for null/undefined or unexpected object shapes to avoid potential runtime errors.
388-388: Arrow function creation on every render.
This inline arrow function is redefined for each row, which is typical in React, but be mindful of any performance impacts for very large tables. Caching or memoizing might be worth considering if performance becomes an issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
shesha-reactjs/src/components/dataTable/interfaces.ts(1 hunks)shesha-reactjs/src/components/reactTable/index.tsx(3 hunks)shesha-reactjs/src/components/reactTable/interfaces.ts(2 hunks)shesha-reactjs/src/designer-components/dataTable/table/models.ts(2 hunks)shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts(4 hunks)shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-test
🔇 Additional comments (10)
shesha-reactjs/src/designer-components/dataTable/table/tableWrapper.tsx (1)
125-125: Looks good - double-click action configuration added correctlyThe new
onDblClickprop is successfully passed to the DataTable component, allowing it to handle double-click actions based on the provided configuration.shesha-reactjs/src/designer-components/dataTable/table/models.ts (2)
2-2: Import added correctlyThe import of
IConfigurableActionConfigurationis properly added to support the new double-click action functionality.
19-19: Double-click action configuration property added correctlyThe new
dblClickActionConfigurationproperty extends the interface appropriately, maintaining consistency with the changes in other files.shesha-reactjs/src/components/reactTable/interfaces.ts (2)
1-1: Import added correctlyThe import of
IConfigurableActionConfigurationis properly added to support the enhanced double-click functionality.
160-160: Enhanced double-click functionalityThe
onRowDoubleClickproperty has been successfully updated to accept either a configurable action or a callback function, providing more flexibility for handling double-click events.shesha-reactjs/src/components/dataTable/interfaces.ts (1)
70-70: Double-click property added correctlyThe
onDblClickproperty has been successfully added to theIShaDataTableInlineEditablePropsinterface, consolidating double-click handling functionality and allowing for either a configurable action or a callback function.shesha-reactjs/src/components/reactTable/index.tsx (1)
25-25: Import usage is valid.
No issues with importing multiple hooks from the same provider.shesha-reactjs/src/designer-components/dataTable/table/tableSettings.ts (3)
123-141: Security tab additions look good.
The newpermissionsinput correctly aligns with the intended security model.
143-256: Appearance tab additions are consistent.
The new fields (minHeight,maxHeight,containerStyle,tableStyle,noDataIcon, etc.) provide useful styling and empty-state options. No immediate concerns.
504-507: Label change is clear.
Renaming to “On Row Save” matches the corresponding logic. No issues.
| { | ||
| key: 'security', | ||
| title: 'Security', | ||
| key: 'events', | ||
| title: 'Events', | ||
| id: securityTabId, | ||
| components: [ | ||
| ...new DesignerToolbarSettings() | ||
| .addSettingsInput({ | ||
| .addConfigurableActionConfigurator({ | ||
| id: nanoid(), | ||
| propertyName: 'permissions', | ||
| label: 'Permissions', | ||
| inputType: 'permissions', | ||
| parentId: securityTabId, | ||
| jsSetting: true, | ||
| tooltip: 'Enter a list of permissions that should be associated with this component', | ||
| propertyName: "dblClickActionConfiguration", | ||
| parentId: 'root', | ||
| label: "On Double Click", | ||
| jsSetting: false, | ||
| readOnly: { _code: 'return getSettingValue(data?.readOnly);', _mode: 'code', _value: false } as any, | ||
| }) | ||
| .addConfigurableActionConfigurator({ | ||
| id: nanoid(), | ||
| propertyName: 'onRowSaveSuccessAction', | ||
| label: 'On Row Save Success', | ||
| parentId: crudTabId, | ||
| description: 'Custom business logic to be executed after successfull saving of new/updated row.', | ||
| hideLabel: true, | ||
| jsSetting: true, | ||
| }) | ||
| .toJson() | ||
| ] | ||
| } |
There was a problem hiding this comment.
Duplicate tab ID for "events" and "security".
The 'events' tab reuses securityTabId, which can cause collisions or incorrect tab rendering. Each tab should have a unique id.
Below is a proposed fix:
-{
- key: 'events',
- title: 'Events',
- id: securityTabId,
- components: [
- ...
- ]
-}
+{
+ key: 'events',
+ title: 'Events',
+ id: nanoid(), // or a dedicated eventsTabId variable
+ components: [
+ ...
+ ]
+}📝 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.
| { | |
| key: 'security', | |
| title: 'Security', | |
| key: 'events', | |
| title: 'Events', | |
| id: securityTabId, | |
| components: [ | |
| ...new DesignerToolbarSettings() | |
| .addSettingsInput({ | |
| .addConfigurableActionConfigurator({ | |
| id: nanoid(), | |
| propertyName: 'permissions', | |
| label: 'Permissions', | |
| inputType: 'permissions', | |
| parentId: securityTabId, | |
| jsSetting: true, | |
| tooltip: 'Enter a list of permissions that should be associated with this component', | |
| propertyName: "dblClickActionConfiguration", | |
| parentId: 'root', | |
| label: "On Double Click", | |
| jsSetting: false, | |
| readOnly: { _code: 'return getSettingValue(data?.readOnly);', _mode: 'code', _value: false } as any, | |
| }) | |
| .addConfigurableActionConfigurator({ | |
| id: nanoid(), | |
| propertyName: 'onRowSaveSuccessAction', | |
| label: 'On Row Save Success', | |
| parentId: crudTabId, | |
| description: 'Custom business logic to be executed after successfull saving of new/updated row.', | |
| hideLabel: true, | |
| jsSetting: true, | |
| }) | |
| .toJson() | |
| ] | |
| } | |
| { | |
| key: 'events', | |
| title: 'Events', | |
| id: nanoid(), // or a dedicated eventsTabId variable | |
| components: [ | |
| ...new DesignerToolbarSettings() | |
| .addConfigurableActionConfigurator({ | |
| id: nanoid(), | |
| propertyName: "dblClickActionConfiguration", | |
| parentId: 'root', | |
| label: "On Double Click", | |
| jsSetting: false, | |
| readOnly: { _code: 'return getSettingValue(data?.readOnly);', _mode: 'code', _value: false } as any, | |
| }) | |
| .addConfigurableActionConfigurator({ | |
| id: nanoid(), | |
| propertyName: 'onRowSaveSuccessAction', | |
| label: 'On Row Save Success', | |
| parentId: crudTabId, | |
| description: 'Custom business logic to be executed after successfull saving of new/updated row.', | |
| hideLabel: true, | |
| jsSetting: true, | |
| }) | |
| .toJson() | |
| ] | |
| } |
1 participant
Summary by CodeRabbit
dblClickActionConfigurationadded to support double-click actions in table components.onDblClickprop introduced for theDataTablecomponent to handle double-click actions based on configuration.