Updated context center icons#29775
Conversation
❌ 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 |
|
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! |
|
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! |
Rohit0301
force-pushed
the
context-center-icons
branch
from
6cea855 to
4f90d46
Compare
|
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! |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| </Tooltip> | ||
| <ButtonUtility | ||
| color="tertiary" | ||
| data-testid="edit-memory-btn" |
There was a problem hiding this comment.
💡 Quality: Duplicate/misleading data-testid 'edit-memory-btn' on manage buttons
Several dropdown trigger buttons for 'manage' actions were given data-testid="edit-memory-btn". In ArticleDetailHeader this replaces the previous data-testid="manage-button", which may break existing Playwright/unit selectors. The id is also semantically wrong on the document/article manage triggers (it is a manage/more-actions button, not an edit-memory button) and collides with the genuine edit-memory button used elsewhere in MemoriesView, making test selection ambiguous. Use a descriptive, unique testid such as manage-button.
Restore/give a descriptive testid for the manage dropdown trigger.:
data-testid="manage-button"
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
Rohit0301
added
the
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| data-testid="edit-memory-btn" | ||
| icon={ <DotsVerticalIcon height={18} width={18} />} | ||
| size="sm" | ||
| tooltip={t('label.manage-entity', { | ||
| entity: t('label.article'), | ||
| })} |
There was a problem hiding this comment.
data-testid="edit-memory-btn", overwriting the previous data-testid="manage-button". Any Playwright test that locates this button by manage-button will silently fail to find it. The same copy-paste issue appears in DocumentsView.component.tsx where the dots button for document management also receives edit-memory-btn.
| data-testid="edit-memory-btn" | |
| icon={ <DotsVerticalIcon height={18} width={18} />} | |
| size="sm" | |
| tooltip={t('label.manage-entity', { | |
| entity: t('label.article'), | |
| })} | |
| data-testid="manage-button" | |
| icon={ <DotsVerticalIcon height={18} width={18} />} | |
| size="sm" | |
| tooltip={t('label.manage-entity', { | |
| entity: t('label.article'), | |
| })} |
| 'tw:bg-primary tw:text-fg-quaternary tw:shadow-xs-skeuomorphic tw:ring-1 tw:ring-primary tw:ring-inset tw:hover:bg-primary_hover tw:hover:text-fg-quaternary_hover tw:disabled:shadow-xs tw:disabled:ring-disabled_subtle', | ||
| tertiary: | ||
| 'tw:text-fg-quaternary tw:hover:bg-primary_hover tw:hover:text-fg-quaternary_hover', | ||
| 'tw:text-quaternary tw:hover:bg-primary_hover tw:hover:text-utility-gray-600', |
There was a problem hiding this comment.
tw:hover:text-utility-gray-600 is a raw palette value, not a semantic design token. Per the project's styling guidelines, hardcoded color values should be replaced with semantic tokens so the component respects light/dark mode and theme changes automatically.
| 'tw:text-quaternary tw:hover:bg-primary_hover tw:hover:text-utility-gray-600', | |
| 'tw:text-quaternary tw:hover:bg-primary_hover tw:hover:text-fg-quaternary_hover', |
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!
❌ UI Checkstyle Failed❌ ESLint + Prettier + Organise Imports (src)One or more source files have linting or formatting issues. Affected files
Fix locally (fast - only checks files changed in this branch): make ui-checkstyle-changed |
🔴 Playwright Results — 23 failure(s), 21 flaky✅ 4444 passed · ❌ 23 failed · 🟡 21 flaky · ⏭️ 54 skipped
Genuine Failures (failed on all attempts)❌
|
Code Review
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar
Code Review 👍 Approved with suggestions 2 resolved / 3 findingsMigrates ContextCenter and KnowledgeCenter icons to custom SVG assets and updates ButtonUtility styles. Please resolve the duplicate 'edit-memory-btn' testid usage across management components. 💡 Quality: Duplicate/misleading data-testid 'edit-memory-btn' on manage buttons📄 openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/ArticleDetailHeader/ArticleDetailHeader.component.tsx:600 📄 openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/DocumentsView/DocumentsView.component.tsx:171 📄 openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/MemoriesView/MemoriesView.component.tsx:54 Several dropdown trigger buttons for 'manage' actions were given Restore/give a descriptive testid for the manage dropdown trigger.✅ 2 resolved✅ Bug: Typo 'ttw:shrink-0' breaks Tailwind class on delete icon
✅ Bug: Follow button shows inverted active/inactive icon
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
1 similar comment
Code Review 👍 Approved with suggestions 2 resolved / 3 findingsMigrates ContextCenter and KnowledgeCenter icons to custom SVG assets and updates ButtonUtility styles. Please resolve the duplicate 'edit-memory-btn' testid usage across management components. 💡 Quality: Duplicate/misleading data-testid 'edit-memory-btn' on manage buttons📄 openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/ArticleDetailHeader/ArticleDetailHeader.component.tsx:600 📄 openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/DocumentsView/DocumentsView.component.tsx:171 📄 openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/MemoriesView/MemoriesView.component.tsx:54 Several dropdown trigger buttons for 'manage' actions were given Restore/give a descriptive testid for the manage dropdown trigger.✅ 2 resolved✅ Bug: Typo 'ttw:shrink-0' breaks Tailwind class on delete icon
✅ Bug: Follow button shows inverted active/inactive icon
🤖 Prompt for agentsOptionsDisplay: 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:
Fixes #
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
ContextCenterandKnowledgeCentercomponents to standardize the icon set.ButtonUtilitystyles and iconography to improve visual consistency and alignment across the UI.This will update automatically on new commits.
Greptile Summary
This PR standardizes iconography across the ContextCenter and KnowledgeCenter components by replacing
@untitledui/iconslibrary icons with custom SVG assets, and updatesButtonUtility's tertiary style tokens.height/widthprops instead of the library'ssizeprop.ButtonUtilitystyle update: The tertiary variant's default text color changes fromtw:text-fg-quaternarytotw:text-quaternaryand the hover text color is hardcoded totw:hover:text-utility-gray-600(non-semantic token).createIconWithStroke-wrapped library icons.Confidence Score: 4/5
Safe to merge once the open follow/unfollow icon inversion and data-testid collision issues from prior review cycles are addressed; the icon replacement itself is straightforward and low-risk.
The follow/unfollow icon state remains inverted from prior review feedback (users always see the wrong star icon), and multiple distinct buttons share data-testid='edit-memory-btn' making automated tests unreliable. The rest of the icon swap is mechanical and low-risk.
ArticleDetailHeader.component.tsx still carries the inverted follow icon logic and the ttw:shrink-0 typo from the prior review cycle.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[ButtonUtility icon prop] --> B{isReactComponent?} B -->|Yes - component ref| C["Render <Icon data-icon />"] B -->|No| D{isValidElement?} D -->|Yes - JSX element| E["Render element as-is\n(no data-icon)"] D -->|No| F[Render nothing] C --> G["✅ Gets data-icon styles:\n• shrink-0\n• size-4/5\n• text-current\n• transition-inherit-all"] E --> H["⚠️ Misses data-icon styles:\n• No auto-shrink\n• No auto-size\n• No transition"] I["Old pattern:\nicon={DotsVertical}"] --> B J["New pattern (this PR):\nicon={<DotsVerticalIcon h=18 w=18 />}"] --> D%%{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[ButtonUtility icon prop] --> B{isReactComponent?} B -->|Yes - component ref| C["Render <Icon data-icon />"] B -->|No| D{isValidElement?} D -->|Yes - JSX element| E["Render element as-is\n(no data-icon)"] D -->|No| F[Render nothing] C --> G["✅ Gets data-icon styles:\n• shrink-0\n• size-4/5\n• text-current\n• transition-inherit-all"] E --> H["⚠️ Misses data-icon styles:\n• No auto-shrink\n• No auto-size\n• No transition"] I["Old pattern:\nicon={DotsVertical}"] --> B J["New pattern (this PR):\nicon={<DotsVerticalIcon h=18 w=18 />}"] --> DReviews (3): Last reviewed commit: "minor fix" | Re-trigger Greptile