-
Notifications
You must be signed in to change notification settings - Fork 1.6k
ui: right entity panel code cleanup #25020
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Code Review 👍 Approved with suggestionsWell-executed migration from LESS stylesheets to MUI's sx prop styling system with comprehensive test updates. Two minor typo/quality issues found. Suggestions 💡 2 suggestionsBug: Typo in sx prop: "paddding" instead of "padding"📄 openmetadata-ui/src/main/resources/ui/src/components/common/DataProductsSection/DataProductsSection.tsx:1252 In sx={{
...
paddding: '3px 0', // Should be "padding"
...
}}This won't cause a runtime error but the padding won't be applied as expected. Suggested fix: Change Code Quality: Missing theme dependency in tagsContent useMemo📄 openmetadata-ui/src/main/resources/ui/src/components/common/TagsSection/TagsSection.tsx:290 In Looking at line 1336, the dependency array for Similarly, in However, it would be cleaner to include What Works WellClean migration pattern consistently applied across all affected components (TagsSection, OwnersSection, GlossaryTermsSection, DescriptionSection, DataProductsSection). Test files properly updated with ThemeProvider wrappers and improved test-id based selectors replacing CSS class queries. Good cleanup of unused DataAssetSummaryPanel component and 6 LESS files. OptionsAuto-apply is off Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs) |
|



Describe your changes:
Fixes
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>Summary by Gitar
DataAssetSummaryPaneldirectory (328 lines) consolidating toDataAssetSummaryPanelV1onlyBoxwithsxprop:DescriptionSection,CommonEntitySummaryInfoV1,DataProductsSection,OwnersSection,GlossaryTermsSection,TagsSectionDescriptionSection.less,OverviewSection.less,DataProductsSection.less,OwnersSection.less,GlossaryTermsSection.less,TagsSection.less(~774 lines total)anytypes withMergedEntityDataandTypeinterfaces inEntitySummaryPanel.component.tsxdata-testidselectors instead of CSS class selectors for better test resilienceThis will update automatically on new commits.