refactor date field and form settings #3396
Conversation
WalkthroughThe changes restore a previously removed input for additional fields to fetch in the data load settings, replace hardcoded string IDs with dynamically generated IDs using Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsEditor
participant FormSettingsConfig
User->>SettingsEditor: Open form settings modal
SettingsEditor->>FormSettingsConfig: Load updated settings configuration
FormSettingsConfig-->>SettingsEditor: Return settings with restored input and dynamic IDs
User->>SettingsEditor: Modify settings and save
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
shesha-reactjs/src/components/formDesigner/formSettings.ts (1)
81-86:⚠️ Potential issueHard-coded
parentIdlooks wrong
parentId: 'qzAhO4Zbgao4mbi7CPJsAbJ5Hw7brZ'isn’t defined anywhere and differs from thedataLoaderTypeIdnanoid declared above. Using an orphan ID will break the component tree. Replace withdataLoaderTypeId.- parentId: "qzAhO4Zbgao4mbi7CPJsAbJ5Hw7brZ", + parentId: dataLoaderTypeId,
🧹 Nitpick comments (2)
shesha-reactjs/src/components/mainLayout/styles/indexStyles.ts (1)
92-98: Risky!importantoverrides – tighten the selector
!importanton core Ant classes inside.sha-form-settings-editorcan unexpectedly affect any future component that re-uses this wrapper. Prefer limiting the scope (e.g. target a specific form item with an additional class) or replacedisplay: inlinewith more granular flex/grid styling to avoid breaking multiline inputs.shesha-reactjs/src/components/formDesigner/formSettingsEditor.tsx (1)
43-44: Min-width 590 px may break mobile UXThe modal now fails to fit ≤ 576 px viewports. Consider keeping the old 320 px floor or switch to Ant’s responsive
wrapClassName/responsiveprops so that settings remain reachable on phones.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
shesha-reactjs/src/components/formDesigner/formSettings.ts(1 hunks)shesha-reactjs/src/components/formDesigner/formSettingsEditor.tsx(2 hunks)shesha-reactjs/src/components/mainLayout/styles/indexStyles.ts(1 hunks)shesha-reactjs/src/designer-components/dateField/settingsForm.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-attempt
🔇 Additional comments (4)
shesha-reactjs/src/components/formDesigner/formSettingsEditor.tsx (1)
58-58: Good move adding a scoping classTagging the form with
sha-form-settings-editorenables targeted styling and avoids global leakage.shesha-reactjs/src/designer-components/dateField/settingsForm.ts (2)
246-250:❓ Verification inconclusive
Renamed
disabledDateFunc– ensure backward-compatExisting saved layouts still persist
isDateDisabled. Provide a migration step or fallback (e.g. read old key if new one is absent) to avoid silently dropping user logic.
🏁 Script executed:
#!/bin/bash # Find remaining references to the old key rg -n "isDateDisabled"Length of output: 22
Ensure legacy
isDateDisabledsettings are handledI ran a search for the old key but found no references to
isDateDisabledin the codebase. Please manually verify that any persisted layouts using this legacy property are correctly migrated or that the loader falls back to the old key whendisabledDateFuncis absent.
292-295:✅ Verification successful
Same concern for
disabledTimeFuncDouble-check that no runtime code still expects
isTimeDisabled, and consider auto-migration logic.
🏁 Script executed:
#!/bin/bash rg -n "isTimeDisabled"Length of output: 22
No occurrences of
isTimeDisabledfound – rename is safe
A global search returned no references toisTimeDisabledin the codebase, so the switch todisabledTimeFuncwon’t break any runtime logic. You can remove the old property and proceed without adding migration code.shesha-reactjs/src/components/formDesigner/formSettings.ts (1)
154-173: 👍 Nice restoration offieldsToFetchRe-adding the autocomplete for extra GraphQL fields closes a long-standing gap.
…into derik-refactor/bug/dateField
refactor form settings and date field
Summary by CodeRabbit
New Features
Enhancements
Style
Refactor