moved some props to data tab #3313
Conversation
WalkthroughThe changes reorganize the placement of entity selection-related input fields within a settings form, moving them from the "Common" tab to the "Data" tab. The "Selection Type" input type is changed from a query builder to a dropdown. A tooltip is added to the "Dialog Width" input to clarify acceptable CSS units. The Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (4)
✨ 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/designer-components/entityPicker/settingsForm.ts(2 hunks)
🔇 Additional comments (3)
shesha-reactjs/src/designer-components/entityPicker/settingsForm.ts (3)
109-109: LGTM! Dynamic container ID generation is appropriate.Using
nanoid()for the container ID is a good practice for ensuring unique identifiers.
112-148: Good logical reorganization of data-related inputs.Moving the Selection Type and Entity Type inputs to the Data tab makes logical sense as these are core data configuration settings.
352-352: Excellent UX improvement with helpful tooltip.The tooltip clearly explains the CSS unit options and default behavior, which will help users configure the dialog width correctly.
shesha-reactjs/src/designer-components/entityPicker/settingsForm.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
shesha-reactjs/src/designer-components/inputComponent/index.tsx (1)
201-205: Dynamic modelType evaluation implemented correctly.The QueryBuilder case now properly evaluates the
modelTypestring expression using the current form data and passes it as a prop. The implementation follows the same pattern as other string evaluations in the component.Consider adding error handling for the
modelTypeevaluation to make it more robust:case 'queryBuilder': { - const modelType = props.modelType ? evaluateString(props.modelType, { data: formData }) : undefined; + const modelType = props.modelType + ? evaluateString(props.modelType, { data: formData }) + : undefined; return <QueryBuilder {...props} hideLabel={true} defaultValue={defaultValue} readOnly={props.readOnly} modelType={modelType} />; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
shesha-reactjs/src/designer-components/entityPicker/settingsForm.ts(7 hunks)shesha-reactjs/src/designer-components/inputComponent/index.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- shesha-reactjs/src/designer-components/entityPicker/settingsForm.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-attempt
🔇 Additional comments (2)
shesha-reactjs/src/designer-components/inputComponent/index.tsx (2)
47-47: Form data access pattern updated correctly.The destructuring pattern change from
{ data: formData }to{ formData }aligns with the new hook implementation. The usage throughout the component correctly wrapsformDatain{ data: formData }when needed for evaluation functions.
6-6:Details
❓ Verification inconclusive
Verify the hook replacement is properly implemented.
The import has been updated to replace
useFormDatawithuseShaFormInstance. Ensure that this hook replacement is consistent across the codebase and thatuseShaFormInstanceprovides the same functionality.
🏁 Script executed:
#!/bin/bash # Description: Verify that useShaFormInstance hook is properly implemented and used consistently # Expected: Find the hook definition and check for any remaining useFormData usage # Check if useShaFormInstance is properly exported from the index rg -A 5 "export.*useShaFormInstance" # Check for any remaining useFormData imports that might have been missed rg "useFormData" --type tsx --type ts # Find the implementation of useShaFormInstance to understand its interface ast-grep --pattern 'export const useShaFormInstance = $_'Length of output: 164
🏁 Script executed:
#!/bin/bash set -e # Search all occurrences of the new hook to verify its definition and usage echo "=== Searching for useShaFormInstance definitions and usage ===" grep -R -n "useShaFormInstance" . # Search for any remaining references to the old hook echo "=== Searching for useFormData references ===" grep -R -n "useFormData" .Length of output: 177
Missing
useShaFormInstanceHook DefinitionI wasn’t able to locate any definition or export of
useShaFormInstancein the codebase. Please verify the following before merging:• Confirm that
useShaFormInstanceis implemented and exported (e.g., in your rootindex.tsor equivalent entry point).
• Remove any remaininguseFormDatareferences to avoid accidental imports.
• Ensure the new hook’s API matches the old one (i.e., it returnsformDataand any other expected fields).
2 participants
#2542
Summary by CodeRabbit