Description untitled UI rewrite#28926
Conversation
…iptionV1 Replace the legacy antd DescriptionV1 (Space/ExpandableCard/IconButtons + description-v1.less) with a new `Description` component built on @openmetadata/ui-core-components (Card/Divider/Typography/Button/Tooltip + @untitledui/icons). The card drops the collapse chevron, sizes to its content, renders the body at 12px, and shows an "Authored by X • time" footer when description change metadata exists. - Migrate all ~40 call sites (+ test mocks) from DescriptionV1 to Description; delete DescriptionV1.tsx and description-v1.less. - Add a compact `xxs` size variant to the core Button for the header icons. - Remove 4 obsolete "card collapse" Playwright tests (chevron gone); add a focused Description unit test. Preserve all testids (asset-description-container, edit-description, description-thread, request-description). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
siddhant1
requested review from
a team,
chirag-madlani and
karanh37
as code owners
❌ 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! |
siddhant1
added
UI
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| return ( | ||
| <EntityAttachmentProvider entityFqn={entityFqn} entityType={entityType}> | ||
| <Space | ||
| {...(wrapInCard | ||
| ? {} | ||
| : { | ||
| 'data-testid': 'asset-description-container', | ||
| })} | ||
| className={classNames('schema-description d-flex', className)} | ||
| direction="vertical" | ||
| size={16}> | ||
| {wrapInCard ? null : header} | ||
| <div> | ||
| <div | ||
| className={classNames( | ||
| wrapInCard | ||
| ? 'tw:flex tw:flex-col tw:gap-4 tw:rounded-[10px] tw:border tw:border-gray-blue-100 tw:bg-bg-primary tw:p-[18px] tw:shadow-xs' | ||
| : 'tw:flex tw:flex-col tw:gap-4', | ||
| className | ||
| )} | ||
| data-testid="asset-description-container"> | ||
| {header} | ||
| <div className="tw:[&_.tiptap.ProseMirror]:text-xs tw:[&_.tiptap.ProseMirror]:leading-[18px]"> | ||
| {descriptionContent} | ||
| {!suggestionData && shouldShowDescriptionMetadata && ( | ||
| <div className="description-v1-metadata"> | ||
| <DescriptionSourceBadge | ||
| changeSummaryEntry={changeSummary?.['description']} | ||
| showBadge={false} | ||
| /> | ||
| </div> | ||
| )} | ||
| <ModalWithMarkdownEditor | ||
| header={t('label.edit-description-for', { entityName })} | ||
| placeholder={t('label.enter-entity', { | ||
| entity: t('label.description'), | ||
| })} | ||
| value={description} | ||
| visible={Boolean(isEditDescription)} | ||
| onCancel={handleCancelEditDescription} | ||
| onSave={handleDescriptionChange} | ||
| /> | ||
| </div> | ||
| </Space> | ||
| {!suggestionData && shouldShowDescriptionMetadata && ( |
There was a problem hiding this comment.
💡 Quality: wrapInCard description card is no longer collapsible
Previously DescriptionV1 rendered the wrapInCard variant inside ExpandableCard (cardProps={{ title: header, className: 'new-description-card' }}), giving the whole description card an expand/collapse affordance. The rewrite replaces this with a static div (tw:rounded-[10px] tw:border ...) and ExpandableCard is no longer imported. As a result every place that passed wrapInCard (CommonWidgets description widget, TestCaseResultTab, TableQueryRightPanel, ClassificationDetails, DocumentationTab, TeamDetailsV1, TestSuiteDetailsPage, etc.) loses the collapse/expand control.
If this UX change is intentional for the Untitled UI rewrite, it can be ignored; otherwise it is an unintended regression. Worth confirming with design and the linked issue.
Was this helpful? React with 👍 / 👎
| <div | ||
| className={classNames( | ||
| wrapInCard | ||
| ? 'tw:flex tw:flex-col tw:gap-4 tw:rounded-[10px] tw:border tw:border-gray-blue-100 tw:bg-bg-primary tw:p-[18px] tw:shadow-xs' | ||
| : 'tw:flex tw:flex-col tw:gap-4', | ||
| className | ||
| )} | ||
| data-testid="asset-description-container"> |
There was a problem hiding this comment.
💡 Quality: Hardcoded/non-semantic Tailwind values in Description card
The project styling guideline requires semantic CSS variables/tokens and discourages hardcoded values. The new card wrapper uses a non-semantic palette color and arbitrary pixel values: tw:border-gray-blue-100, tw:rounded-[10px], and tw:p-[18px]. Other classes in the same component correctly use semantic tokens (tw:bg-bg-primary, tw:text-text-secondary). Consider replacing gray-blue-100 with a semantic border token and the arbitrary radius/padding with the design-system scale (e.g. tw:rounded-xl, tw:p-4/tw:p-5) to stay consistent with the token guideline.
Was this helpful? React with 👍 / 👎
Code Review
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar
Jest test CoverageUI tests summary
|
|
🟡 Playwright Results — all passed (16 flaky)✅ 4264 passed · ❌ 0 failed · 🟡 16 flaky · ⏭️ 88 skipped
🟡 16 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 |
Describe your changes:
Fixes #
Rewrites the entity Description card as a new
Descriptioncomponent built entirely on the Untitled UI design system (@openmetadata/ui-core-components), aligning the description box with the 2.0 Release design (Figma node8286-5231).What changed
Descriptioncard built with coreCard/Divider/Typography/Button/Tooltip+@untitledui/iconsandtw:design tokens: a white card with a gray-blue border, soft shadow and 10px radius; a "Description" header with compact edit / request / comment icon buttons; a 12px rich-text body; a divider; and an "Authored by {user} • {time}" footer (shown when the description has tracked change metadata).DescriptionV1(Space/ExpandableCard/ antd IconButtons +description-v1.less) and migrated all ~40 call sites to the newDescription.xxssize variant to the coreButtonfor the header icon buttons.Why
Brings the most-used entity surface onto the 2.0 design system, removes the Ant Design dependency for this component, and produces a cleaner, content-sized card (no collapse chevron).
No functional regressions — the rich-text body, edit modal, suggestions, tasks, comment thread, version diff and attachments are reused unchanged (
RichTextEditorPreviewerV1,ModalWithMarkdownEditor,DescriptionSourceBadge,EntityAttachmentProvider,SuggestionsAlert/SuggestionsSlider). All test ids are preserved (asset-description-container,edit-description,description-thread,request-description).Type of change:
High-level design:
components/common/EntityDescription/Description.tsx(new, sameDescriptionProps, no.less): ports all ofDescriptionV1's logic verbatim (hooks, handlers, memos) and only swaps the presentation to Untitled UI.wrapInCardtoggles the card chrome vs. an inline layout.RichTextEditorPreviewerV1) and edit modal (ModalWithMarkdownEditor) are reused as-is; only the card, header, divider, footer and action buttons are rebuilt in core-components.DescriptionV1→Descriptionacross ~40 source files + their test mocks;DescriptionV1.tsxanddescription-v1.lessdeleted. Props are identical, so each call site is a pure rename.Button: new additivexxssize for the compact header icons (existing sizes unchanged).ExpandableCardand sizes to its content via the existingDynamicHeightWidget. The 4 obsolete "card collapse" Playwright tests were removed and a focusedDescription.test.tsxwas added.Tests:
Use cases covered
Unit tests
Description.test.tsx(title, edit-button gating incl. version view, previewer rendering, footer).DocumentationTab(renders the real component) and theGenericTab/DynamicHeightWidgetgrid tests.Backend integration tests
Ingestion integration tests
Playwright (UI) tests
playwright/e2e/Pages/DescriptionVisibility.spec.ts(removed 4 obsolete "card collapse" tests; the read-more / scroll tests are retained).DescriptionVisibility.spec.ts8/8 andMarkdown.spec.ts2/2.Manual testing performed
tsc --noEmitis clean and the affected Jest suites pass.UI screen recording / screenshots:
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.