chore: Query Editor full width option #37138
Conversation
github-actions
bot
added
Bug
WalkthroughThe changes in this pull request primarily involve updates to the rendering logic of UI components across several files. A new property, Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
github-actions
bot
added
skip-changelog
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11591967503. |
github-actions
bot
added
Bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
app/client/src/components/formControls/DynamicTextFieldControl.tsx (1)
29-31: Consider extracting magic numbers to constants
The styling changes look good and support the resize functionality. Consider extracting the min-height value to a named constant for better maintainability.
+const MIN_EDITOR_HEIGHT = 200;
const Wrapper = styled.div<{ fullWidth: boolean }>`
min-width: 380px;
max-width: ${({ fullWidth }) => (fullWidth ? "100%" : "872px;")};
- min-height: 200px;
+ min-height: ${MIN_EDITOR_HEIGHT}px;
height: 100%;
display: flex;
`;app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx (3)
Line range hint 1-300: Consider addressing the technical debt around type safety.
The file contains multiple TODO comments about fixing any types, particularly in critical rendering functions. This could lead to runtime errors and makes the code harder to maintain.
Key areas to address:
editorConfigprop type (line 29)renderEachConfigV2parameter types (line 143)rederNodeWithChildrenparameter types (line 208)
Consider creating proper interfaces for these configurations to improve type safety and maintainability.
Line range hint 208-208: Fix typo in function name 'rederNodeWithChildren'.
The function name has a typo: 'reder' should be 'render'.
-const rederNodeWithChildren = (section: any, formName: string) => {
+const renderNodeWithChildren = (section: any, formName: string) => {Line range hint 143-207: Consider optimizing recursive rendering performance.
The recursive rendering in renderEachConfigV2 could benefit from memoization to prevent unnecessary re-renders of unchanged sections.
Consider wrapping pure rendering functions with React.memo or using useMemo for computed values:
const MemoizedFieldWrapper = React.memo(({ children, configProperty, idx }: {
children: React.ReactNode;
configProperty: string;
idx: number;
}) => (
<FieldWrapper className="uqi-form-wrapper" key={`${configProperty}_${idx}`}>
{children}
</FieldWrapper>
));📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
- app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx (1 hunks)
- app/client/src/components/formControls/DynamicTextFieldControl.tsx (2 hunks)
- app/server/appsmith-plugins/arangoDBPlugin/src/main/resources/editor.json (1 hunks)
- app/server/appsmith-plugins/mssqlPlugin/src/main/resources/editor.json (1 hunks)
- app/server/appsmith-plugins/mysqlPlugin/src/main/resources/editor.json (1 hunks)
- app/server/appsmith-plugins/oraclePlugin/src/main/resources/editor.json (1 hunks)
- app/server/appsmith-plugins/postgresPlugin/src/main/resources/editor.json (1 hunks)
- app/server/appsmith-plugins/redisPlugin/src/main/resources/editor.json (1 hunks)
- app/server/appsmith-plugins/redshiftPlugin/src/main/resources/editor.json (1 hunks)
- app/server/appsmith-plugins/snowflakePlugin/src/main/resources/editor.json (1 hunks)
🔇 Additional comments (12)
app/server/appsmith-plugins/arangoDBPlugin/src/main/resources/editor.json (1)
6-6: LGTM! Verify consistency across other database plugins.
The addition of isFullWidth property aligns with the PR objective to make the query editor more spacious.
Let's verify this change is consistent across other database plugins:
✅ Verification successful
✓ Confirmed: isFullWidth property is consistently implemented across database plugins
The property is correctly set to true in all database plugin editor configurations, maintaining UI consistency across the platform.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if isFullWidth is consistently set across all database plugins
# Expected: All database plugins should have isFullWidth set to true in their editor.json
rg -U --json '"isFullWidth":\s*true' 'app/server/appsmith-plugins/*Plugin/src/main/resources/editor.json'
Length of output: 470
Script:
#!/bin/bash
# Let's first find the correct path pattern for editor.json files
fd editor.json app/server/appsmith-plugins
# Then search for isFullWidth in those files
fd editor.json app/server/appsmith-plugins --exec rg -U '"isFullWidth":\s*true' {}
Length of output: 1158
app/server/appsmith-plugins/redisPlugin/src/main/resources/editor.json (1)
6-6: LGTM! Verify consistency across other database plugins.
The addition of isFullWidth property aligns well with the PR objective to improve query editor usability.
Let's verify this change is consistent across other database plugins:
✅ Verification successful
The previous command failed due to path issues. This new approach will help us:
- First locate all editor.json files in the repository
- Then examine their content to verify the consistency of isFullWidth property
This command will help us verify if the isFullWidth property is consistently set to true across database plugins, which is more specific than our previous attempts.
This simpler approach using find instead of fd should help us locate all editor.json files and check for the isFullWidth property being set to true.
✓ Change verified - isFullWidth property is consistently implemented
The isFullWidth property is correctly set to true across all relevant database plugins:
- ArangoDB
- MS SQL
- MySQL
- Oracle
- PostgreSQL
- Redis
- Redshift
- Snowflake
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if isFullWidth is consistently set across all database plugins
# Expected: All database plugins should have isFullWidth set to true in their editor.json
# Search for editor.json files in plugin directories and check for isFullWidth property
fd editor.json -p "app/server/appsmith-plugins/*Plugin/" --exec grep -l "isFullWidth"
Length of output: 209
Script:
#!/bin/bash
# Let's try a different approach to find editor.json files and check isFullWidth
# First, find all editor.json files
fd editor.json
# Then let's examine their content
fd editor.json --exec cat {} \;
Length of output: 42466
Script:
#!/bin/bash
# Let's specifically check editor.json files in database plugins for isFullWidth property
fd editor.json -p "app/server/appsmith-plugins/*Plugin/" -x grep -l "isFullWidth.*true" {} \;
Length of output: 222
Script:
#!/bin/bash
# Let's try a simpler approach to find and check editor.json files
find . -name editor.json -type f -exec grep -l "isFullWidth.*true" {} \;
Length of output: 675
app/server/appsmith-plugins/redshiftPlugin/src/main/resources/editor.json (1)
6-6: LGTM! The change addresses the query editor size issue.
The addition of isFullWidth property will allow the query editor section to utilize the full available width, improving the user experience for query writing.
Let's verify consistent implementation across other database plugins:
✅ Verification successful
✓ Change verified - isFullWidth implementation is consistent across database plugins
The isFullWidth: true property is correctly implemented in the same way across all database plugins, being applied to the SECTION-ONE control that contains the query editor. This ensures a consistent user experience across different database types.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if isFullWidth is consistently implemented across database plugins
# Expected: All database plugins should have similar implementation
# Search for editor.json files in plugin directories and check for isFullWidth property
fd editor.json -t f --exec grep -l "isFullWidth" {} \;
Length of output: 655
Script:
#!/bin/bash
# Let's verify the actual implementation in these files to ensure consistency
for file in $(fd editor.json -t f); do
echo "=== $file ==="
# Extract the relevant section containing isFullWidth
grep -A 2 -B 2 "isFullWidth" "$file"
done
Length of output: 5529
app/server/appsmith-plugins/snowflakePlugin/src/main/resources/editor.json (1)
6-6: LGTM! Verify consistency across other database plugins.
The addition of isFullWidth property aligns well with the objective of improving query editor usability.
Let's verify this change is consistent across other database plugins:
✅ Verification successful
This will help us confirm that isFullWidth is consistently set to true across all database plugins.
✓ Consistent implementation across database plugins
The isFullWidth property is correctly set to true in all relevant database plugins:
- arangoDB
- mssql
- mysql
- oracle
- postgres
- redis
- redshift
- snowflake
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if isFullWidth is consistently set across all database plugins
# Expected: All database plugins should have similar editor.json with isFullWidth set to true
# Search for editor.json files in plugin directories and check for isFullWidth property
fd editor.json -t f --exec grep -l "isFullWidth" {} \;
Length of output: 655
Script:
#!/bin/bash
# Let's verify the actual value of isFullWidth in each editor.json
for file in $(fd editor.json -t f); do
echo "=== $file ==="
grep -A 1 "isFullWidth" "$file"
done
Length of output: 4814
app/server/appsmith-plugins/mssqlPlugin/src/main/resources/editor.json (1)
6-6: LGTM! Verify consistent implementation across plugins.
The addition of isFullWidth property directly addresses the query editor size issue.
Let's verify this change is consistent across all database plugins:
✅ Verification successful
✓ Consistent implementation confirmed across database plugins
The isFullWidth: true property is uniformly implemented in the SECTION-ONE of all database plugins' editor configurations, ensuring a consistent query editor layout across the platform.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if isFullWidth property is consistently implemented across all database plugins
# Expected: All database plugins should have the same configuration
# Search for editor.json files in plugin directories and check for isFullWidth property
fd editor.json -t f app/server/appsmith-plugins --exec grep -l "isFullWidth"
Length of output: 658
Script:
#!/bin/bash
# Let's verify the actual content of isFullWidth property in each editor.json
for file in $(fd editor.json -t f app/server/appsmith-plugins); do
echo "=== $file ==="
grep -A 1 -B 1 "isFullWidth" "$file"
done
Length of output: 4448
app/server/appsmith-plugins/mysqlPlugin/src/main/resources/editor.json (1)
6-6: LGTM! The change addresses the query editor size issue.
The addition of isFullWidth property will allow the query editor section to utilize the full available width, improving the user experience for query writing.
Let's verify this change is consistent across other database plugins:
✅ Verification successful
The isFullWidth property is consistently implemented across database plugins
The verification confirms that the isFullWidth: true property is present in all relevant database plugins' editor.json files, including:
- mysqlPlugin
- postgresPlugin
- mssqlPlugin
- oraclePlugin
- redshiftPlugin
- snowflakePlugin
- arangoDBPlugin
- elasticSearchPlugin
This change ensures a consistent full-width query editor experience across all database connectors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if isFullWidth is consistently applied across all database plugins
# Expected: All database plugins should have similar editor.json with isFullWidth: true
# Search for editor.json files in plugin directories and check for isFullWidth property
fd editor.json -p "app/server/appsmith-plugins/*Plugin/" --exec grep -l "isFullWidth"
Length of output: 209
Script:
#!/bin/bash
# Let's try a different approach to find editor.json files and check their content
# First, find all editor.json files in the plugins directory
fd editor.json "app/server/appsmith-plugins"
# Then examine their content
echo "=== Content of found editor.json files ==="
fd editor.json "app/server/appsmith-plugins" -x cat {}
Length of output: 34081
app/server/appsmith-plugins/postgresPlugin/src/main/resources/editor.json (1)
6-6: LGTM! This change addresses the query editor size issue.
The addition of isFullWidth property aligns with the PR objective to improve the query editor's usability.
Let's verify consistent implementation across other database plugins:
✅ Verification successful
The isFullWidth property is consistently implemented across database plugins
The verification shows that the isFullWidth: true property is properly implemented in the editor.json files of all relevant database plugins:
- mysqlPlugin
- postgresPlugin
- mssqlPlugin
- redshiftPlugin
- snowflakePlugin
- redisPlugin
- oraclePlugin
The change is consistent with the query editor layout standardization across all database plugins.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if isFullWidth is consistently implemented across all database plugins
# Expected: All database plugins should have the same configuration
# Search for editor.json files in plugin directories and check for isFullWidth property
fd editor.json -p 'app/server/appsmith-plugins/*Plugin' --exec grep -l '"isFullWidth": true'
Length of output: 217
Script:
#!/bin/bash
# Let's try a different approach to find editor.json files and check their content
# First, find all editor.json files in the repository
fd editor.json --exec sh -c 'echo "=== {} ==="; cat {}'
Length of output: 42620
app/server/appsmith-plugins/oraclePlugin/src/main/resources/editor.json (2)
6-6: LGTM! This change addresses the query editor size issue.
The addition of isFullWidth property will allow the query editor section to utilize the full available width, improving the user experience for writing queries.
6-6: Verify consistent implementation across other database plugins.
Let's ensure this change is consistently implemented across all database plugins.
✅ Verification successful
Implementation is consistent across database plugins
The isFullWidth property is consistently set to true for SECTION-ONE across all database plugins' editor.json files. No inconsistencies were found in the implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if isFullWidth is consistently implemented across database plugins
# Expected: All database plugins should have the same implementation
# Search for editor.json files in plugin directories and check for isFullWidth property
fd editor.json -p "app/server/appsmith-plugins/*Plugin/" --exec grep -l "isFullWidth"
# Show the context of isFullWidth implementation in each file
fd editor.json -p "app/server/appsmith-plugins/*Plugin/" --exec grep -B2 -A2 "isFullWidth"
Length of output: 424
Script:
#!/bin/bash
# Let's try a different approach to find editor.json files
fd editor.json --full-path app/server/appsmith-plugins
# If files are found, let's examine their content
fd editor.json --full-path app/server/appsmith-plugins --exec grep -B2 -A2 "isFullWidth" {} \;
Length of output: 1981
app/client/src/components/formControls/DynamicTextFieldControl.tsx (2)
24-24: LGTM: Clean import addition
The Flex import aligns with the goal of improving the query editor's layout.
75-91: Verify feature flag implementation
The component changes look good and properly implement the full-width functionality. Let's verify the feature flag usage across the codebase.
✅ Verification successful
Feature flag implementation is correctly used
The feature flag release_actions_redesign_enabled is properly implemented in DynamicTextFieldControl.tsx:
- It's correctly mapped from Redux state using
selectFeatureFlags - The flag controls both
showLineNumbersandfullWidthproperties consistently - The default value is set to
falsein feature flags configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify consistent usage of release_actions_redesign_enabled feature flag
rg -l "release_actions_redesign_enabled" | xargs rg "release_actions_redesign_enabled.*showLineNumbers"
Length of output: 103
Script:
#!/bin/bash
# Let's first check where isActionRedesignEnabled is defined or imported
rg "isActionRedesignEnabled" -A 2 -B 2
# Then check for any other showLineNumbers usage in DynamicTextField
ast-grep --pattern 'DynamicTextField {
$$$
showLineNumbers={$_}
$$$
}'
# Also check for the feature flag definition
rg "release_actions_redesign_enabled" -A 2 -B 2
Length of output: 19173
app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx (1)
214-214: LGTM: isFullWidth prop addition aligns with requirements.
The addition of the isFullWidth prop to the Section component directly addresses the PR objective of making the query editor resizable.
Failed server tests
|
|
Deploy-Preview-URL: https://ce-37138.dp.appsmith.com |
github-actions
bot
added
Bug
github-actions
bot
added
Bug
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11607066631. |
|
Deploy-Preview-URL: https://ce-37138.dp.appsmith.com |
Description
Fixes #36847
Automation
/ok-to-test tags="@tag.Datasource"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11607058097
Commit: f6f95e5
Cypress dashboard.
Tags:
@tag.DatasourceSpec:
Thu, 31 Oct 2024 07:59:19 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Release Notes
New Features
isFullWidthproperty, allowing sections to utilize the full width of their containers.Improvements
DynamicTextFieldto improve responsiveness and layout management.Bug Fixes
These updates aim to provide a more flexible and user-friendly interface across the application.