Merged
Conversation
Contributor
WalkthroughThis update refactors the data list settings form by removing nested collapsible panels and reorganizing related settings into a flatter structure with direct input rows. Additionally, new margin styles are added for Ant Design collapse headers and alerts in the styles file. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsForm
participant UI
User->>SettingsForm: Open Data Tab
SettingsForm->>UI: Render direct input rows for add/edit/delete settings
User->>SettingsForm: Change "Can Add/Edit/Delete Inline"
SettingsForm->>UI: Show/hide related input rows based on selection
User->>SettingsForm: Open Events Tab
SettingsForm->>UI: Show "onListItemSaveSuccessAction" event setting
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
shesha-reactjs/src/components/dataList/styles/styles.ts (1)
24-30: Consider alternatives to!importantfor better maintainability.While the margin adjustments for Ant Design components support the flattened form structure, using
!importantcan make styles harder to maintain and override in the future.Consider these alternatives:
- Increase CSS specificity by adding more selectors
- Use CSS custom properties (CSS variables) for consistent spacing
- Apply styles via CSS-in-JS with higher specificity
If
!importantis necessary due to Ant Design's default styles, consider adding comments explaining why it's required:.ant-collapse>.ant-collapse-item >.ant-collapse-header { - margin: 4px !important; + margin: 4px !important; /* Override Ant Design default to improve spacing in flattened form */ } .ant-alert { - margin: 5px !important; + margin: 5px !important; /* Override Ant Design default for consistent spacing */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
shesha-reactjs/src/components/dataList/styles/styles.ts(1 hunks)shesha-reactjs/src/designer-components/dataList/settingsForm.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: teboho
PR: shesha-io/shesha-framework#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.
Learnt from: teboho
PR: shesha-io/shesha-framework#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.
shesha-reactjs/src/components/dataList/styles/styles.ts (3)
Learnt from: teboho
PR: shesha-io/shesha-framework#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.
Learnt from: teboho
PR: shesha-io/shesha-framework#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.
Learnt from: teboho
PR: shesha-io/shesha-framework#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.
shesha-reactjs/src/designer-components/dataList/settingsForm.ts (3)
Learnt from: teboho
PR: shesha-io/shesha-framework#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.
Learnt from: teboho
PR: shesha-io/shesha-framework#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.
Learnt from: teboho
PR: shesha-io/shesha-framework#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.
🔇 Additional comments (4)
shesha-reactjs/src/designer-components/dataList/settingsForm.ts (4)
197-312: Well-structured refactoring of "Can Add Inline" settings.The flattening of the nested collapsible panel structure successfully addresses the styling issues while maintaining all functionality. The conditional visibility logic is preserved correctly, and the settings are now more accessible in the flattened hierarchy.
Key improvements:
- Cleaner structure with direct input rows instead of nested panels
- Preserved all conditional logic for form selection modes
- Maintained proper parent-child relationships
314-368: Consistent refactoring approach for "Can Edit Inline" settings.The edit settings follow the same successful pattern as the add settings, with proper conditional visibility and good organization of related inputs into logical rows.
369-395: Simple and effective refactoring of "Can Delete Inline" settings.The delete settings maintain the same pattern, appropriately simplified since there are fewer related settings compared to add/edit operations.
423-429: Proper relocation of save action to Events tab.Moving the "On List Item Save Action" configurable action from the data tab to the events tab is a logical improvement that better groups related functionality. This enhances the overall organization of the settings form.
James-Baloyi
approved these changes
Jul 18, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#3119 (comment)
Summary by CodeRabbit
Refactor
Style