Migrated TagsV1 and TagsViewer components#29778
Conversation
Rohit0301
added
the
safe to test
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <Typography | ||
| ellipsis | ||
| className="m-0" | ||
| data-testid="add-tag" | ||
| ellipsis={{ tooltip: false }}> | ||
| size="text-xs" | ||
| weight="medium"> | ||
| {getTagDisplay(tagName)} | ||
| </Typography.Text> | ||
| </Typography> |
There was a problem hiding this comment.
text-primary colour class removed from "add tag" chip
The old Typography.Text carried text-primary (among other classes) that explicitly set the text colour to the brand primary. The new <Typography> only sets size and weight, with no color prop or text-primary class, so the text will fall back to whatever colour the <Tag> ancestor injects — which may not match the expected primary/blue colour. If the "add tag" chip text renders in the wrong colour after this change, adding a color prop or restoring the text-primary className here is the fix.
| <Tooltip | ||
| delay={500} | ||
| key={tag.tagFQN} | ||
| placement="bottom-start" | ||
| slotProps={{ | ||
| tooltip: { | ||
| sx: { | ||
| bgcolor: 'common.black', | ||
| color: 'common.white', | ||
| }, | ||
| }, | ||
| }} | ||
| title={getTagTooltip(tag.tagFQN, tag.description) ?? ''}> |
There was a problem hiding this comment.
placement prop dropped — tooltip position changes silently
The old <MuiTooltip> used placement="bottom-start", which anchored the tooltip to the bottom-left of the tag chip. The new <Tooltip> has no placement prop, so it will fall back to whatever the @openmetadata/ui-core-components default is. In a dense list of tags this could cause the tooltip to overlap adjacent chips or appear in an unexpected direction. Consider passing placement="bottom-start" (or the equivalent prop name) to preserve the original behaviour.
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Jest test CoverageUI tests summary
|
🟡 Playwright Results — all passed (26 flaky)✅ 4462 passed · ❌ 0 failed · 🟡 26 flaky · ⏭️ 54 skipped
🟡 26 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Code Review ✅ Approved 1 resolved / 1 findingsMigrates ✅ 1 resolved✅ Bug: TooltipTrigger (a ) wraps navigable anchors
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Code Review ✅ Approved 1 resolved / 1 findingsMigrates ✅ 1 resolved✅ Bug: TooltipTrigger (a ) wraps navigable anchors
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
1 participant
Describe your changes:
Uploading Screen Recording 2026-07-06 at 6.44.03 PM.mov…
Fixes 4867
I worked on ... because ...
Type of change:
High-level design:
N/A — small change.
Tests:
Use cases covered
Unit tests
Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
UI screen recording / screenshots:
Not applicable.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.Summary by Gitar
TagsV1andTagsViewerto use custom components from@openmetadata/ui-core-components.MuiTooltipwithTooltipandTooltipTriggerfor enhanced design consistency.TagsV1to useBadgefor automated tags and refined styling usingtw:utility classes.Typographycomponents and icons inTagsV1for better alignment and responsiveness.This will update automatically on new commits.
Greptile Summary
This PR migrates
TagsV1andTagsViewercomponents from MUI (@mui/material) and Antd primitives to the internal@openmetadata/ui-core-componentslibrary, replacingMuiTooltipwith the customTooltip,Typography.TextwithTypography, andTagChip(MUI-based) withBadgefor automated tags.TagsV1: RemovesuseTheme, replacesTagChipwithBadgefor automated tag rendering, wraps theLinkinsidetagChipwith<Focusable>, and updates icon classes fromflex-shrinktotw:shrink-0. TheuseMemodependency arrays are also corrected to include previously missing deps (isVersionPage,newLook,tagChipStyleClass).TagsViewer: ReplacesMuiTooltipwith the newTooltipin themuiTagsrendering branch, changes the tooltip placement frombottom-starttotop, and switches the link width class fromw-fulltotw:w-max.Confidence Score: 3/5
Two tooltip trigger wiring issues could silently break tooltip display for ordinary and automated tags before merging.
The new
Tooltipfrom@openmetadata/ui-core-componentsrequires aTooltipTriggerchild (confirmed byEntityTitleSection.tsxusage). Both changed files use<Focusable>instead, and the regulartagChipcode path inTagsV1passes the Antd<Tag>directly — with no trigger wrapper at all. This means tag tooltips may not activate on hover or keyboard focus in these two paths after the migration.TagsV1.component.tsx(regular tag tooltip path at line 312–317) andTagsViewer.tsx(muiTags tooltip path at lines 59–85) both need their tooltip trigger children corrected from<Focusable>/ direct child to<TooltipTrigger>.Important Files Changed
@openmetadata/ui-core-components;tagChipis passed to<Tooltip>without a<TooltipTrigger>or<Focusable>wrapper, inconsistent withautomatedTagChipand the library's documented pattern.MuiTooltipwithTooltipfrom@openmetadata/ui-core-componentsin themuiTagsrender path; uses<Focusable>as the tooltip trigger child where<TooltipTrigger>is the expected pattern per other usages in the codebase.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[TagsViewer] -->|muiTags=true| B[Tooltip from ui-core-components] A -->|muiTags=false| C[TagsV1] B --> D["<Focusable> ⚠️ should be <TooltipTrigger>"] D --> E[Link → TagChip] C --> F{labelType === Generated?} F -->|yes + isEditTags| G[automatedTagChip - Link → Badge] F -->|yes + tooltip| H["Tooltip → <Focusable> → automatedTagChip"] F -->|no + isEditTags| I[tagChip - Tag → Focusable → Link] F -->|no + tooltip| J["Tooltip → tagChip ⚠️ missing trigger wrapper"] style D fill:#ffe0b2,stroke:#e65100 style J fill:#ffe0b2,stroke:#e65100%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[TagsViewer] -->|muiTags=true| B[Tooltip from ui-core-components] A -->|muiTags=false| C[TagsV1] B --> D["<Focusable> ⚠️ should be <TooltipTrigger>"] D --> E[Link → TagChip] C --> F{labelType === Generated?} F -->|yes + isEditTags| G[automatedTagChip - Link → Badge] F -->|yes + tooltip| H["Tooltip → <Focusable> → automatedTagChip"] F -->|no + isEditTags| I[tagChip - Tag → Focusable → Link] F -->|no + tooltip| J["Tooltip → tagChip ⚠️ missing trigger wrapper"] style D fill:#ffe0b2,stroke:#e65100 style J fill:#ffe0b2,stroke:#e65100Comments Outside Diff (2)
openmetadata-ui/src/main/resources/ui/src/components/Tag/TagsV1/TagsV1.component.tsx, line 107-114 (link)tagsV1.lessline 107 has a rule targeting.tags-tooltip.MuiTooltip-tooltip … p { color: @grey-700; }. That rule fired because the old MUI<Tooltip>receivedslotProps={{ tooltip: { className: 'tags-tooltip' } }}, which caused MUI to attach both.tags-tooltipand.MuiTooltip-tooltipto the tooltip element. Both classes are now gone, so the compound selector never matches — anypelement inside a tag tooltip that contains markdown/rich text will revert to the default colour instead ofgrey-700.openmetadata-ui/src/main/resources/ui/src/components/Tag/TagsV1/TagsV1.component.tsx, line 257-273 (link)addTagChip—<Tag>and the opening<Typography>have an extra leading space compared to the surrounding code, which breaks the expected 2-space indent.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (4): Last reviewed commit: "lint fix" | Re-trigger Greptile