refactor(ui): standardize all entity icons to Phosphor and fix sizing#16285
refactor(ui): standardize all entity icons to Phosphor and fix sizing#16285annadoesdesign wants to merge 1 commit intomasterfrom
Conversation
Migrate all V2 entity type icons from Ant Design/MUI to @phosphor-icons/react for consistency and dark mode readiness. Fix icon sizing inconsistencies across search results, entity profiles, browse paths, and document mentions. - Convert all 28 entity type icon() methods to use Phosphor icons - Convert getSubTypeIcon() from Ant Design to Phosphor (Database, Schema, etc.) - Fix ContextPath icon sizing: subtypes got 1em while entity types got 20px - Fix ContainerIcon CSS font-size override that didn't work on SVGs - Fix getTypeIcon passing '1em' string to numeric fontSize parameter - Standardize all preview typeIcon sizes to 14px - Convert ContextPathEntityIcon browse path icon to Phosphor - Update preview logoComponent props to use Phosphor with size prop - Use semantic theme colors for document mention text (textDisabled, text, icon) - Add IconWrapper for consistent mention icon alignment in documents Co-authored-by: Cursor <cursoragent@cursor.com>
|
🔴 Meticulous spotted visual differences in 1 of 1294 screens tested: view and approve differences detected. Meticulous evaluated ~9 hours of user flows against your PR. Last updated for commit 6787951. This comment will update as new commits are pushed. |
Bundle ReportChanges will decrease total bundle size by 10.39kB (-0.04%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: datahub-react-web-esmAssets Changed:
Files in
|
datahub-cyborg
bot
added
the
needs-review
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
chriscollins3456
left a comment
chriscollins3456
left a comment
There was a problem hiding this comment.
this seems pretty reasonable. will message IRL about a question i have
| const InvalidEntityText = styled.span` | ||
| font-weight: 500; | ||
| color: ${ANTD_GRAY[7]}; | ||
| text-decoration: line-through; |
There was a problem hiding this comment.
so we want a line through invalid entities now?
| margin-left: 4px; | ||
| word-break: break-all; | ||
| color: ${(props) => props.theme.styles['primary-color']}; | ||
| color: ${(props) => props.theme.colors.text}; |
There was a problem hiding this comment.
is text different than the primary color?
| <ShareAltOutlined | ||
| <ShareNetwork | ||
| className={TYPE_ICON_CLASS_NAME} | ||
| style={{ fontSize: fontSize || 'inherit', color: color || 'inherit' }} |
There was a problem hiding this comment.
the biggest change i'm seeing consistently here is we no longer default to inherit and instead default to 20 - is that intended? what changes here?
| <Database size={12} color="currentColor" /> {entityCount} {entityCount === 1 ? 'entity' : 'entities'} | ||
| <StyledDivider /> | ||
| <DomainIcon /> {subDomainCount} {pluralize(subDomainCount, 'sub-domain')} | ||
| <Globe size={12} color="currentColor" /> {subDomainCount} {pluralize(subDomainCount, 'sub-domain')} | ||
| <StyledDivider /> | ||
| <FileDoneOutlined /> {dataProductCount} {pluralize(dataProductCount, 'data product')} | ||
| <Package size={12} color="currentColor" /> {dataProductCount} {pluralize(dataProductCount, 'data product')} |
There was a problem hiding this comment.
it might be nice to use entityRegistry to get the correct entity icons here for domain and data product at least
datahub-cyborg
bot
added
pending-submitter-response
2 participants
Summary
Standardize all V2 entity type icons from Ant Design (
@ant-design/icons) and MUI (@mui/icons-material) to Phosphor (@phosphor-icons/react) for visual consistency and dark mode readiness. Fixes icon sizing inconsistencies across the UI where subtype icons rendered at1emwhile entity type icons rendered at20px.Changes
Entity icons (28 entity types):
icon()methods now return Phosphor icons with consistentsize,color="currentColor", andweightprops@ant-design/iconsand@mui/icons-materialimports from entity icon methodsSubtype icons (
getSubTypeIcon):DatabaseOutlined,ApartmentOutlined,DeploymentUnitOutlined,FilterOutlined,LineChartOutlinedto Phosphor equivalentssizeparameter (previously relied on CSSfont-sizewhich doesn't work on SVGs)Icon sizing fixes:
ContextPath.tsx: Fixed root cause where subtypes got1emsize and entity types got20px— now both use14pxContainerIcon.tsx: Replaced broken CSSfont-size: 14pxon.typeIcon(no effect on SVGs) with explicitsizepropgetTypeIcon.tsx: Changed'1em'string to numeric14for Phosphor compatibilityContextPathEntityIcon.tsx: Converted browse path folder icon fromFolderOpenOutlinedto PhosphorFolderOpentypeIconcalls from inconsistent12px/14pxto14pxPreview file cleanup:
logoComponenticons with matching Phosphor icons in glossary, schema field, business attribute, and glossary node previewsDomainIconwith hardcoded colorDomainEntitiesSnippet: Converted entity count icons to PhosphorDocument mentions:
MentionsNodeView.tsx(both versions): Use semantic theme colors (textDisabled,text,icon) instead of hardcodedANTD_GRAYIconWrapperfor consistent icon alignment in inline mentionsBefore:
After:
Checklist
Made with Cursor