Revert "Scale fields in components (#7213)" #7253
Conversation
This reverts commit 1a4c2d0.
📝 WalkthroughWalkthroughThe pull request includes several updates primarily focused on the dependency management of Material-UI packages across multiple files. In the Minor formatting changes were made in the Possibly related PRs
Suggested labels
Suggested reviewers
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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
designer/submodules/packages/components/src/common/defaultTheme.tsx (1)
75-77: Consider the visual impact of the background color blend.The
blendLightenfunction with a value of 0.12 is being used to create a subtle contrast for the column headers. While this works, consider:
- Testing this in both light and dark themes to ensure sufficient contrast
- Documenting the rationale for the specific blend value of 0.12
designer/client/package.json (1)
Based on the search results, I can now generate the final response:
@mui/system removal is safe with minor adjustments needed
The package is not directly used as a main dependency, but there are specific imports from its submodules that need attention:
getLuminancefrom "@mui/system/colorManipulator" (used in multiple files)blendfrom "@mui/system" (used in theme helpers and styling)These imports should be updated to use equivalent functions from
@mui/materialinstead, as these utilities are also available there. Specific files to update:
- designer/client/src/containers/theme/helpers.ts
- designer/client/src/components/graph/EspNode/element.ts
- designer/client/src/components/ComponentPreview.tsx
- designer/client/src/components/toolbars/activities/helpers/activityItemColors.ts
- designer/client/src/stylesheets/SelectStyled.ts
- And other files using these utilities
🔗 Analysis chain
Line range hint
1-300: Verify impact of @mui/system removalThe removal of
@mui/systempackage needs careful verification as it might break existing styling implementations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for @mui/system usage and potential breaking changes # Search for direct imports of @mui/system rg "from '@mui/system'" --type ts --type tsx -A 3 # Search for specific @mui/system utilities rg "styled|Box|createTheme|ThemeProvider" --type ts --type tsx -A 3 # Look for any remaining references to @mui/system in other package files fd package.json --exec jq -r 'select(.dependencies != null) | select(.dependencies["@mui/system"] != null)'Length of output: 293
Script:
#!/bin/bash # Let's try a different approach to search for @mui/system usage # Search for direct imports without file type restrictions rg "from ['|\"]@mui/system" -A 3 # Search for specific @mui/system utilities without file type restrictions rg "import.*?{.*?(styled|Box|createTheme|ThemeProvider).*?}.*?from ['|\"]@mui/(system|material)" -A 3 # Look for any remaining references to @mui/system in package.json files find . -name "package.json" -type f -exec grep -l "@mui/system" {} \;Length of output: 6140
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
designer/client/package-lock.jsonis excluded by!**/package-lock.jsondesigner/client/test/Process/__snapshots__/ProcessStateIcon-test.tsx.snapis excluded by!**/*.snapdesigner/submodules/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
designer/client/package.json(1 hunks)designer/client/src/components/NodeInput.tsx(1 hunks)designer/submodules/packages/components/package.json(1 hunks)designer/submodules/packages/components/src/common/defaultTheme.tsx(1 hunks)designer/submodules/packages/components/src/components/componentTable.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- designer/client/src/components/NodeInput.tsx
🔇 Additional comments (6)
designer/submodules/packages/components/package.json (2)
12-13: Ensure version parity between @mui/material and @mui/icons-material
The synchronized downgrade to 5.15.7 for both packages maintains version parity, which is good practice.
12-14:
Verify compatibility and impact of MUI package downgrades
The significant version downgrades warrant careful consideration:
- @mui/material & @mui/icons-material: 5.16.7 -> 5.15.7
- @mui/x-data-grid: 7.22.2 -> 6.18.5 (major version downgrade)
Consider documenting:
- The reason for reverting to these specific versions
- Any known compatibility issues or limitations
- Migration steps if breaking changes affect the codebase
The major version downgrade of @mui/x-data-grid (7.x -> 6.x) likely introduces breaking changes. Please ensure:
- All DataGrid usage patterns are compatible with v6.x
- Features specific to v7.x are not being used
- TypeScript types are updated accordingly
✅ Verification successful
Let me gather more information about DataGrid usage patterns with a different approach since the previous typescript file type wasn't recognized.
Let me check the actual DataGrid usage patterns in the affected files to understand the impact.
Based on the gathered evidence, I can now provide a final response:
DataGrid downgrade requires attention to specific usage patterns
The version downgrade from 7.x to 6.x of @mui/x-data-grid impacts several files that use DataGrid features:
- Basic DataGrid usage in
tableWrapper.tsxwith standard props (autoPageSize, rows, getRowHeight) - Custom styling in
defaultTheme.tsxusing MuiDataGrid style overrides - Simple id mapping for DataGrid in
useScenariosQuery.tsx - Basic hover styles in
tableCellAvatar.tsx
The current implementation appears to use basic DataGrid features that are stable across v6 and v7, and no v7-specific features were detected in the codebase. The coordinated downgrade of @mui/material and @mui/icons-material to 5.15.7 maintains version compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes and security advisories
# Search for MUI DataGrid usage patterns that might be affected by the downgrade
echo "Checking for DataGrid usage patterns..."
rg -l "DataGrid|useDataGrid" --type typescript --type typescript-react
# Check for security advisories
echo "Checking for security advisories..."
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@mui/material") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
# Check peer dependency requirements
echo "Checking package.json files for potential conflicts..."
fd package.json --type f --exec jq -r '.dependencies | select(. != null) | with_entries(select(.key | startswith("@mui"))) // empty'
Length of output: 1076
Script:
#!/bin/bash
# Search for DataGrid usage patterns with ripgrep
echo "Checking for DataGrid usage patterns..."
rg -l "DataGrid|useDataGrid"
# Check the specific version changes in git history
echo "Checking version changes in package.json..."
git show HEAD -- designer/submodules/packages/components/package.json
# Look for potential breaking changes in documentation files
echo "Checking for migration or breaking changes documentation..."
rg -i "breaking.*changes|migration|upgrade" -g "*.md"
Length of output: 12438
Script:
#!/bin/bash
# Check DataGrid usage in the identified files
echo "Checking DataGrid implementation details..."
rg -A 5 -B 5 "DataGrid|useDataGrid" designer/submodules/packages/components/src/components/tableWrapper.tsx designer/submodules/packages/components/src/common/defaultTheme.tsx designer/submodules/packages/components/src/scenarios/useScenariosQuery.tsx designer/submodules/packages/components/src/scenarios/list/tableCellAvatar.tsx
# Check for any package-lock.json or yarn.lock to verify dependency resolution
echo "Checking lock files..."
fd "package-lock.json|yarn.lock" --type f --exec head -n 20 {}
Length of output: 9027
designer/submodules/packages/components/src/components/componentTable.tsx (2)
9-9: LGTM! Import statement is correctly maintained.
The import of GridActionsCellItem is necessary and properly used in the component.
18-25: Verify the impact of reduced flex value on name column width.
The flex value for the name column has been significantly reduced from 4 to 1, while other columns maintain larger flex values (e.g., categories with flex: 4). This change might:
- Make the name column too narrow for longer component names
- Create an unbalanced table layout
- Affect overall readability
designer/submodules/packages/components/src/common/defaultTheme.tsx (1)
75-77: Verify the compatibility of the new style with downgraded Material-UI version.
The addition of columnHeadersInner style override might be affected by the Material-UI downgrade mentioned in the PR summary (from 5.16.7 to 5.15.7). Please ensure this styling property is supported in the downgraded version.
✅ Verification successful
The style override is compatible with the downgraded Material-UI version
The columnHeadersInner style is a valid style override for MUI DataGrid in version 5.15.7. The blendLighten function is a custom utility that's consistently used across the codebase for styling purposes, and the style property is applied to a standard background color property. The downgrade from 5.16.7 to 5.15.7 is a minor version change that doesn't affect DataGrid styling APIs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any breaking changes in MUI DataGrid styling between versions
# Search for any DataGrid-related changes in the codebase that might need similar updates
rg -g '*.tsx' -g '*.ts' 'columnHeadersInner'
# Look for any migration guides or documentation files that might contain relevant information
fd -g 'MIGRATION*.md' -g 'CHANGELOG*.md'
Length of output: 306
Script:
#!/bin/bash
# Let's check for any DataGrid-related styling patterns and imports
rg -g '*.tsx' -g '*.ts' 'MuiDataGrid' -A 5 -B 5
# Search for any other usage of blendLighten function
rg -g '*.tsx' -g '*.ts' 'blendLighten'
# Look for package.json to verify MUI versions
fd -g 'package.json' --exec cat {}
Length of output: 28040
designer/client/package.json (1)
14-16: Verify MUI package compatibility after version downgrades
The downgrade of Material-UI packages to earlier versions requires careful verification:
@mui/icons-material: 5.16.7 → 5.15.7@mui/lab: 5.0.0-alpha.173 → 5.0.0-alpha.165@mui/material: 5.16.7 → 5.15.7
Consider the following recommendations:
- Document the reason for the version downgrades in the PR description to help with future maintenance
- Add a package.json comment explaining why specific versions are required
- Consider implementing a version constraint (e.g.,
~5.15.7) to allow patch updates while preventing minor version bumps
✅ Verification successful
Version downgrade is safe to proceed
Based on the analysis of the codebase:
- The
@mui/systempackage removal is safe as its usage is limited to styling utilities that are also available through@mui/material. - The
@mui/labusage is minimal, only importingLoadingButtonwhich has been stable across the versions being changed. - The version changes (5.16.7 → 5.15.7) are minor version downgrades that maintain compatibility within the v5.x series.
- The package versions are consistently aligned across all dependencies in the monorepo.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential peer dependency conflicts and usage patterns
# Check for peer dependency requirements in package-lock.json
fd package-lock.json --exec jq -r '.packages | .[] | select(.dependencies != null) | select(.dependencies["@mui/material"] != null or .dependencies["@mui/icons-material"] != null or .dependencies["@mui/lab"] != null) | {name: .name, dependencies: .dependencies}'
# Look for direct Material-UI component usage
rg -l '@mui/(material|icons-material|lab)' --type ts --type tsx
# Check for @mui/system imports that might break after removal
rg -l '@mui/system' --type ts --type tsx
Length of output: 4266
Script:
#!/bin/bash
# Let's try a different approach to find MUI usage without file type restrictions
# Check for MUI imports
rg -l '@mui/(material|icons-material|lab)'
# Check for @mui/system imports specifically
rg -l '@mui/system'
# Check if there are any explicit version requirements in other package.json files
fd package.json --exec grep -l "@mui/" {} \;
# Look for potential breaking changes by searching for specific MUI components or patterns
rg "import.*@mui/lab" -A 2
Length of output: 20698
This reverts commit 1a4c2d0.
Describe your changes
Checklist before merge
Summary by CodeRabbit
New Features
MuiDataGridcomponent's column headers.Bug Fixes
Refactor
ComponentTablecomponent for better type management and layout adjustments.Style
NodeInputcomponent for cleaner code structure.