Repository navigation
Fixes 34755: Inbox comment composer's format bar in the design's editor style - #35238
harsh-vador wants to merge 66 commits into
Conversation
Review: the inbox restyled core components instead of using them. - Tab counts (Activity/Triage, the Activity sub-tabs, All/Open/Closed) go through Tabs.Item badge; the hand-built badges and the list/item padding, track and outline overrides are gone. - Like and the replies toggle read brand while on through the Button's own link-color (boxed), not recoloured tertiary buttons. - The date and group menus drop their trigger class and size like the Type filter beside them; menu rows show their selection the core way. - Open asset, summary links and the table tile are core link buttons; the title's asset link takes Typography's link style; the forced weights, underlines, badge fonts and the assignee popover height are dropped.
The Triage tab badge counts every visible Open task, but the sidebar bubble still applied the inbox date window, so it undercounted whenever an Open task was older than 30 days (or the selected range). Fixes #34755
… filter A grouping always has a value, so as a FilterSelect its "active filter" tint never cleared and was overridden with tw:**:text-* classes. It is now the single-choice ActivityToolbarMenu (core Dropdown), read "Group: Type", with a quiet tertiary sm trigger through the menu's new color / size props. The Collate E2E's menuitemradio pick and trigger text still match.
…y' into fix/inbox-page-layout-consistency
- Like reads "Liked · N" with a filled thumb once the viewer has liked; no
hover underline (it no longer borrows the link-color button), and its icon
lines up with the body above.
- Reactions on cards and replies are the design's pills: rounded, 26px, the
viewer's own tinted brand, others white with a secondary border, 5px between
emoji and count; a plain smiley (new core FaceSmile icon) adds one. The
classic feed keeps its chips (ReactionsVariant.Default).
- The card header is 14px throughout; the asset link has no underline and
keeps the primary text color over the Typography prose color.
- "Hide replies" sits on a brand-tinted chip; repliers' faces get a white
edge so overlapping ones read apart.
- Reply rows: the hover actions no longer stretch the name row, so the
message sits 4px under the name.
- Composer: stays one line once it holds text (the FeedEditor's 100px minimum
had won over the Inbox's 44px), and lists start at the text edge.
- Lists in any FeedEditor drew two markers ("1•"): the Quill 1 rules in
feed-editor.less still marked each item while Quill 2 marks it on .ql-ui.
- A read-only rich-text preview no longer shows the trailing-node's empty
paragraph after a final list (a ~30px gap under such a comment).
…-consistency # Conflicts: # .github/playwright/impact-map.generated.json
FilledThumbsUp forwarded only className, dropping the data-icon the core Button sets on its icon; its size and color classes key off that attribute, so a liked card's thumb grew from 16px to 20px and lost the brand tint. Forward every prop.
The Playwright matrix filed every task type in every workflow state (about 47 workflow tasks) only to check how the detail panel renders them: the type badge, the type's own approve and reject wording while open, the outcome and no actions once closed, an incident's own transitions. That is pure rendering of the task, so it now lives in a TaskDetailPanel table test, with exact label matches, across 11 types x 4 states and the incident states. InboxTaskMatrix.spec.ts keeps the Tag request in every state as the check against a real workflow, and every test of what an action does to the asset (tag, owner, domain, tier, incidents, reassignment). It seeds a dozen tasks instead of about 60 and drops the glossary setup. Timing-baseline entries for the removed tests are dropped; the impact map is regenerated.
- The asset link on a card reads as a link: brand blue, regular weight. - A plain message sits on the recessed surface of a change's Before. - Like, the reaction smiley and Reply share one 16px icon size and tighter spacing. - A reply's reactions sit apart from its text, and a lone smiley lines up with the text. - The composer's emoji picker opens right above or below its button, which shows as pressed while the picker is open. - A group header has no rule; its update count sits on the right at 12px, and Asset and User groups lead with the asset's icon or the user's picture.
The composer watched the format bar it found on mount. The editor loads lazily and quill rebuilds its bar when it re-initialises, so after a reply the picker opened in a bar nobody watched, back at the composer's far edge. Watch the composer instead and read the bar afresh.
The shared entity icons draw at a 1.5 stroke, heavier than the 1.3 the design system's icons use and than the text beside them. The Inbox's 16px asset icons, on the card and on an Asset group's header, now take the 1.3 stroke through one class.
The design's Activity icons give each of the 16 kinds of activity (a conversation and 15 event types) its own 12px badge glyph and fill; the cards drew six, shared across kinds. The glyphs join the core icon library as activity-*.svg, generated into ActivityAssetCreated and so on, and ACTIVITY_EVENT_BADGE maps every ActivityEventType to one, so a type added to the schema fails the build until it has a badge. An event type the UI does not know yet reads as a plain update. The fills are the nearest utility tokens to the design's colors; moss, teal and the darker blue have no token of their own. The Type filter keeps its icons.
A tier change arrives as a tags change, and a column's tags or
description can arrive as its asset's. The card's sentence already read
those apart ("changed the tier"), but its badge took the event type as
sent and drew the tags badge. getActivityKindType now reads the kind
once, for both.
The task detail's asset card named the asset and offered a separate Open asset button. The name is now the link (brand blue, regular weight, like the activity cards' asset link), keeping the button's test id, and the unused label.open-asset leaves every locale.
…escription change as Existing and Suggested Design review: a card's message and a change's Before column move from bg-secondary_subtle to utility gray-blue 50. A description change reads Existing and Suggested rather than Before and After; tags, owners and domains keep Before and After. Adds label.existing and label.suggested in every locale.
Clicking a task's current assignee in the single-select Reassign picker clears the pick, and the panel answered with 'Assignees is required.' Picking the same person again would have sent a reassign that changes nothing. getReassignBlocker now names who is already assigned in both cases, and keeps 'required' for an empty pick on a task with no assignee. Adds message.already-assigned-to-task in every locale.
…ds do Design review: - A description task shows Existing and Suggested side by side, in the activity cards' change panel, instead of a Suggested description box. Existing is the payload's snapshot, or the field's description on the asset as it is now when the payload carries none. - A tag task shows the tags it removes and adds in the same panel, in place of the two tag rows. - The selected task row is filled with utility blue light 50 and bordered with blue light 100.
The reference shows the tags a field already has, neutral, before and after, with only the added and removed ones colored. A change can now carry the values it keeps; a tag task fills them from the payload's current tags, else the field's tags on the asset as it is now. An activity event still shows only its change, since it records no more.
…ad's are empty Tag tasks carry currentTags: [] by default rather than a snapshot, which hid the field's tags. Only a non-empty snapshot stands in for the asset.
getField read any path it did not recognise as the entity itself, so a
task on a topic's schema field or a container's data-model column showed
the topic's or container's own description as Existing and its tags as
kept. Only entity-level paths ('', description, tags, entity) now read
the entity; columns.<path> follows its names through nested columns
(address.city), drops the quotes of a dotted name ("a.b"), and still
finds a nested column named on its own. Any other path resolves to
nothing, so the panel shows no Existing side rather than a wrong one.
The design system redrew the Activity icons on a 20px grid with a 1.3 stroke. Replaces 14 of the 16 activity-*.svg sources with the new exports and regenerates their components; Asset Deleted and Custom Property Updated were not redrawn and are unchanged. Column Description Updated's pencil drops a white fill that only masked empty space and would show solid on the colored badge.
Design review: a row with several people (two assignees, say) listed each avatar and name on its own line, so the row grew tall and its neighbours fell out of line. Several people now stack in the core AvatarGroup, as owners and incident assignees do: a hover card per avatar and a +N titled by the row's own label. One person still reads as avatar and name, with (You) for the viewer. Rows center their label on the value.
Design review: the field's header row (Description, Tags) sat on a gray fill; it now reads on the panel's own surface, its rule still splitting it from the columns. Shared by the activity cards and the task detail.
🔄 Playwright impact map auto-refreshedThis PR touched specs or UI source that changed the source→spec routing map. I regenerated What is this file? It is the auto-generated half of Playwright's PR planner. It routes "if source X changes, run specs Y" by walking spec imports and cross-referencing What if I want to regenerate locally instead? Run this before pushing your next change to skip the bot commit: python3 .github/scripts/generate_playwright_impact_map.py
git add .github/playwright/impact-map.generated.json
git commit --amend --no-edit # or a separate commit |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const isSameButtons = (a: Array<Element | null>, b: Array<Element | null>) => | ||
| a.length === b.length && a.every((button, index) => button === b[index]); |
There was a problem hiding this comment.
CommentComposer.tsx contains format-bar constants, helpers, and useFormatBar. The repository guide requires component files to contain only components and their props. Move constants and helpers into sibling files, and put the hook in src/hooks/. This repository requirement must be satisfied before merging.
Context Used: AGENTS.md (source)
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!
| 'tw:[&_.editor-container]:rounded-[10px]! tw:[&_.editor-container]:bg-primary!', | ||
| // Quill sets 13px Helvetica; comments read in the page's font at 14px. | ||
| 'tw:[&_.ql-container.ql-snow]:border-none! tw:[&_.ql-container.ql-snow]:bg-transparent!', | ||
| 'tw:[&_.ql-container.ql-snow]:font-[inherit]! tw:[&_.ql-container.ql-snow]:text-[14px]!', |
There was a problem hiding this comment.
The new Tailwind rules use rounded-[10px], text-[14px], and fixed button padding. The styling guide requires shared tokens instead of raw pixel values for spacing, radius, and font size. Replace these values with tokens before merging; the same pattern appears elsewhere in this file.
Context Used: AGENTS.md (source)
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!
| it('saves the edited text from the send button', () => { | ||
| const onSave = jest.fn().mockResolvedValue(undefined); | ||
| mockEditorContent = 'Looking at this tomorrow.'; | ||
| renderEdit(onSave); | ||
|
|
||
| fireEvent.click(screen.getByTestId('send-button')); | ||
|
|
||
| expect(onSave).toHaveBeenCalledWith('Looking at this tomorrow.'); |
There was a problem hiding this comment.
This test supplies mockEditorContent and checks only that onSave receives it, while the store, avatar, and editor are mocked. It can pass even if the real editor cannot read or save an edited comment. The repository guide requires tests of real behavior rather than mocked call wiring. Add a test that keeps the real editor and checks the resulting comment before merging.
Context Used: AGENTS.md (source)
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!
| LINK_TOOLTIP_CLASS | ||
| ); | ||
|
|
||
| // The format bar shows only while the formatting toggle beside send is on. |
There was a problem hiding this comment.
This comment repeats what COLLAPSED_CLASS and its value already say. The repository guide requires comments to explain why, not restate obvious code. Remove it before merging.
| // The format bar shows only while the formatting toggle beside send is on. |
Context Used: AGENTS.md (source)
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!
Main added the it-it locale after this PR added label.show-formatting and label.hide-formatting to every locale, so it-it lacked them and I18n Sync failed.
Code Review ✅ Approved 1 closed / 1 findings🟡 Medium risk · The shared composer changes Inbox comment posting and editing flows, which could disrupt submissions or draft recovery if integration behavior differs. Restyles the Inbox comment composer with the designer's format-bar icons, adds a formatting toggle beside send, and moves edit-comment into the same composer box with the new-line hint left of Cancel. Resolved a dark-mode rule that hid the brand focus colour on focused buttons. No open issues found. ✅ 1 closed✅ Bug: Dark-mode idle rule hides the brand focus colour on focused buttons
VerificationRules No rules evaluated Functional validation Not enabledAuto-approval Not enabledFor more verification details, comment OptionsDisplay: compact → Showing compact verification results. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
This branch was successfully deployed
1 participant
Describe your changes:
Follow-up to #34817 (the Inbox design review). The design review asked for the Inbox comment composer to keep its existing box and switch its format-bar icons to the designer's set.
Comment box (Inbox composer only)
text-quaternary; hover, a toggled format or the open emoji picker show in brand blue.Formatting toggle (design: Comment Composer, 1a "Formatting toggle next to send")
Type01icon, split from send by a hairline) shows the bar above the text and stays highlighted in brand while it shows. Its tooltip reads "Show formatting" / "Hide formatting" (label.show-formatting/label.hide-formatting, translated in every locale).Editing a comment in the same box
CommentEditor, andCommentComposeris the avatar beside it.CommentEditFormopens a comment in that editor, over a row with the new-line hint on the left and Cancel on the right;TaskCommentRowandActivityThreadboth use it, with their test ids unchanged.FeedEditorsets an inlinergba(0, 0, 0, 0.1)border on its container while it is focused. An edit box opens focused, so it lost the gray-blue border and, in dark mode, its edge all but vanished (the composer did the same once clicked into). The editor's scoped border now holds over the inline one.No more
.less: Tailwind, and a generic name.less, soinbox-comment-composer.lessis removed. Its rules are Tailwind classes on the editor's root, grouped by part (box, format bar, text, mention list, link tooltip). Quill builds that markup itself, so they reach it with arbitrary descendant variants (tw:[&_.ql-toolbar]:…), asPersonaFullscreenPortal,EdgeInfoDrawerandCodeEditoralready do for third-party markup. They carry!: Quill's snow theme and the FeedEditor's styles are unlayered CSS, which beats Tailwind's utilities whatever their selector..lessdid implicitly, spelled out:before:/after:also resetcontent, so the link tooltip's::before/::after("Enter Url:", "Save") are reached inside the selector; Tailwind reads_as a space in those selectors, so the icon marker class iscomment-composer-icon; the theme'sshadow-lgis empty here, so the mention list keeps--om-shadow-lg; the format groups'vertical-align: middlewas Quill's own already, so that rule is dropped.components/common/CommentComposerasCommentComposer,CommentEditorandCommentEditForm. The oldInboxPage/components/InboxCommentComposerpath re-exports the default so Collate keeps building until it imports the new path (open-metadata/openmetadata-collate#7546, to merge after this; then the re-export goes). Theinbox-comment-composertest id stays: OSS and Collate specs find the composer by it.Icons
yarn icons:generate:TextEditorBold,Italic,Strikethrough,Blockquote,Code,NumberedList,BulletedList,Link,Mention,HashtagandEmoji.stroke, which would have thickened a filled path.Scope: why portals
ui/icons). Changing them there would restyle every editor: the classic Activity Feed, the Task tab, the thread panel and Collate chat.MutationObserveron the composer, already there for the emoji picker, tracks the bar. The bar loads lazily, is rebuilt when Quill re-initialises, and gains the emoji button last.Emoji picker fix
topon every pass, but the first pass clears thattop. Any later change in the bar (now the icons; before, switching an emoji tab) re-anchored an above picker below it. The side is now read once per picker.postgres run link - https://github.com/open-metadata/OpenMetadata/actions/runs/38906790106
Type of change:
Tests:
CommentComposer.test.tsx: the design's icon is drawn into each format button (Quill's and quill-emoji's), including into a bar rebuilt after mount. The existing emoji-anchoring tests pass with the read-once side.CommentComposer.test.tsx: the format bar starts hidden, and the toggle shows and hides it;CommentEditFormopens the comment in the composer's editor with send enabled, shows the new-line hint beside Cancel and saves from send.main..less→ Tailwind, checked for regressions against the.lessversion, light and dark, in ten states (collapsed, format bar open, a hovered button, focused with text, the @ mention list, the link tooltip, the emoji picker, the edit box, the edit box with its bar, the thread composer): every computed style of every element in the composer, and of each::before/::after, is the same; screenshots match pixel for pixel but for sub-perceptual antialiasing (≤ 4/255 on a few icon edges) and the edited comment's timestamp.Features/AIMode/InboxTasks.spec.tsandFeatures/AIMode/InboxActivity.spec.ts(including posting, editing and deleting a task comment and a reply): 32 passed against a local server, again after the Tailwind migration.yarn ui-checkstyle:changedis clean in the UI and in core-components.#EAECF5) in light and neutral in dark, and the emoji picker opens above its button. Light and dark: the composer is one line with the toggle; the toggle shows the bar and turns brand; an edited comment opens in the same box with the hint left of Cancel, and keeps the composer's border while focused (#EAECF5light,rgb(55, 58, 65)dark).Checklist:
🤖 Generated with Claude Code
The latest translations look safe, but the four outstanding repository requirements must be addressed before merging.
Findings
Summary
The PR gives the shared comment editor designer icons, a formatting toggle, and Tailwind styles.
Reviews (8) · Last reviewed commit: "i18n: translate the formatting toggle la..." · Reviewed by Greptile