Skip to content

chore(message-parser): fix missing color token in attachment field description#39923

Open
thekishandev wants to merge 1 commit intoRocketChat:developfrom
thekishandev:chore/fix-missing-color-token-field-39922
Open

chore(message-parser): fix missing color token in attachment field description#39923
thekishandev wants to merge 1 commit intoRocketChat:developfrom
thekishandev:chore/fix-missing-color-token-field-39922

Conversation

@thekishandev
Copy link
Copy Markdown

@thekishandev thekishandev commented Mar 28, 2026

Proposed changes

Removes a technical debt marker from the fallback Field component by properly enforcing text color token inheritance via Fuselage boundaries.

Root Cause / Problem

The value node (the description text of the attachment field) was being rendered raw, without a corresponding typography container assigning an explicit color token.

// BEFORE
// TODO: description missing color token
const Field = ({ title, value, ...props }: FieldProps) => (
	<Box mb={4} pi={4} width='full' flexBasis={100} flexShrink={0} color='default' {...props}>
		<Box fontScale='p2m'>{title}</Box>
		{value}
	</Box>
);

Solution / Behavior
Wrapped the raw {value} node in a standard Fuselage to guarantee strict semantic rendering and prevent arbitrary overrides across custom themes. Removed the obsolete TODO comment.

// AFTER
const Field = ({ title, value, ...props }: FieldProps) => (
	<Box mb={4} pi={4} width='full' flexBasis={100} flexShrink={0} color='default' {...props}>
		<Box fontScale='p2m'>{title}</Box>
		<Box color='default'>{value}</Box>
	</Box>
);

Issue(s)
Closes #39922

Validation & Testing
Tested locally (UI components render correctly)
No compilation errors
No runtime behavior changed (pure structural A11y / styling fix)

Type of change
chore: small task (technical debt removal)

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed color rendering for message field values to use the default color token explicitly instead of inheriting colors from parent containers, ensuring consistent color display.

@thekishandev thekishandev requested a review from a team as a code owner March 28, 2026 11:42
Copilot AI review requested due to automatic review settings March 28, 2026 11:42
@dionisio-bot
Copy link
Copy Markdown
Contributor

dionisio-bot bot commented Mar 28, 2026

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 28, 2026

⚠️ No Changeset found

Latest commit: 7397a1c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR resolves a theme consistency issue in the default attachment Field renderer by ensuring the attachment field description (value) is rendered within a Fuselage Box that explicitly sets a text color token, and removes the now-obsolete TODO marker.

Changes:

  • Removed the technical-debt TODO about a missing color token in the attachment field description.
  • Wrapped the value node in a <Box color="default"> to enforce explicit color token application for field descriptions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5d731d0a-0a97-473e-8326-390901148494

📥 Commits

Reviewing files that changed from the base of the PR and between 4235cd9 and 7397a1c.

📒 Files selected for processing (1)
  • apps/meteor/client/components/message/content/attachments/default/Field.tsx
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • apps/meteor/client/components/message/content/attachments/default/Field.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.

Applied to files:

  • apps/meteor/client/components/message/content/attachments/default/Field.tsx
📚 Learning: 2026-03-20T13:51:23.302Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.

Applied to files:

  • apps/meteor/client/components/message/content/attachments/default/Field.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.

Applied to files:

  • apps/meteor/client/components/message/content/attachments/default/Field.tsx
🔇 Additional comments (1)
apps/meteor/client/components/message/content/attachments/default/Field.tsx (1)

13-13: Looks good — explicit color token is correctly applied to value.

This satisfies the stated objective and keeps behavior/API stable.


Walkthrough

A TODO comment about missing color token was removed from the Field component, and the value ReactNode was wrapped in a <Box color='default'> element to explicitly apply the default color token for design system compliance.

Changes

Cohort / File(s) Summary
Color Token Fix
apps/meteor/client/components/message/content/attachments/default/Field.tsx
Removed TODO comment and wrapped value prop in <Box color='default'> to apply explicit color token for consistent theme rendering.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested labels

type: chore

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately reflects the main change: fixing a missing color token in the attachment field description by wrapping the value in a Box component and removing the TODO.
Linked Issues check ✅ Passed The PR successfully addresses both requirements from issue #39922: removes the TODO comment and wraps the value with an explicit Fuselage Box color token.
Out of Scope Changes check ✅ Passed All changes are directly scoped to resolving issue #39922; only the Field.tsx file was modified with minimal, targeted changes to apply the color token fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(message-parser): fix missing color token in attachment field description

2 participants