chore: Stop hardcoding tooltips on message actions #37127
Conversation
|
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
WalkthroughAdds optional tooltip to MessageActionConfig. Removes hardcoded/automatic disabled-tooltips in toolbar menus. Menus now include a tooltip only when provided by options. Permalink and Reply-in-DM actions now set a tooltip when content is encrypted via translations. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant TM as ToolbarMenu
participant AO as ActionOptions
participant I18N as i18n
U->>TM: Open message actions
TM->>AO: Read option { disabled, tooltip?, ... }
alt tooltip provided
TM-->>U: Render item with tooltip
else no tooltip provided
TM-->>U: Render item without tooltip
end
sequenceDiagram
participant Hook as usePermalinkAction / useReplyInDMAction
participant Ctx as Message Context
participant I18N as i18n
Ctx-->>Hook: { encrypted }
alt encrypted === true
Hook->>I18N: t('Action_not_available_encrypted_content', { action })
I18N-->>Hook: localized string
Hook-->>Ctx: { ..., disabled: true, tooltip }
else
Hook-->>Ctx: { ..., disabled: false, (no tooltip) }
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37127 +/- ##
========================================
Coverage 67.39% 67.40%
========================================
Files 3332 3332
Lines 113521 113524 +3
Branches 20606 20607 +1
========================================
+ Hits 76513 76519 +6
+ Misses 34399 34398 -1
+ Partials 2609 2607 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/meteor/client/components/message/toolbar/MessageToolbarActionMenu.tsx (1)
117-140: Consider extracting encrypted apps section logic.The logic for handling encrypted apps sections (lines 117-140) is duplicated in
MessageToolbarStarsActionMenu.tsx(lines 56-66). Both files construct the same "Unavailable" item with the same tooltip when content is encrypted.Consider extracting this into a shared helper function:
// In a shared utilities file export const createEncryptedAppsSectionPlaceholder = (t: TFunction, id: string) => ({ id: 'apps', title: t('Apps'), items: [ { content: t('Unavailable'), type: 'apps', id, disabled: true, gap: false, tooltip: t('Action_not_available_encrypted_content', { action: t('Apps') }), }, ], });Then use it in both files:
if (isMessageEncrypted && section.id === 'apps') { - return { - id: 'apps', - title: t('Apps'), - items: [ - { - content: t('Unavailable'), - type: 'apps', - id, - disabled: true, - gap: false, - tooltip: t('Action_not_available_encrypted_content', { action: t('Apps') }), - }, - ], - }; + return createEncryptedAppsSectionPlaceholder(t, id); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/meteor/app/ui-utils/client/lib/MessageAction.ts(1 hunks)apps/meteor/client/components/message/toolbar/MessageToolbarActionMenu.tsx(1 hunks)apps/meteor/client/components/message/toolbar/MessageToolbarStarsActionMenu.tsx(1 hunks)apps/meteor/client/components/message/toolbar/usePermalinkAction.ts(1 hunks)apps/meteor/client/components/message/toolbar/useReplyInDMAction.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
🔇 Additional comments (8)
apps/meteor/app/ui-utils/client/lib/MessageAction.ts (1)
26-26: LGTM!The optional
tooltip?: stringfield correctly extends the type surface to support runtime-translated tooltip strings. The type aligns with how tooltips are populated in the action hooks (translated strings fromt()calls).apps/meteor/client/components/message/toolbar/MessageToolbarStarsActionMenu.tsx (2)
40-40: LGTM!The conditional tooltip spreading correctly implements the PR objective by removing the hardcoded tooltip for disabled actions and allowing individual actions to provide their own tooltips.
56-66: Approve tooltip localization for encrypted apps The translation keyAction_not_available_encrypted_contentexists in all locales and correctly interpolatesaction.apps/meteor/client/components/message/toolbar/MessageToolbarActionMenu.tsx (1)
102-102: LGTM!The conditional tooltip spreading correctly removes the hardcoded tooltip behavior and allows actions to provide their own tooltips, consistent with the PR objective.
apps/meteor/client/components/message/toolbar/useReplyInDMAction.ts (3)
4-4: LGTM!The
useTranslationimport is correctly added to support tooltip translations.
21-21: LGTM!The translation hook is correctly initialized for use in the tooltip.
75-75: Approve conditional encrypted-content tooltip
Translation keysAction_not_available_encrypted_contentandReply_in_direct_messagehave been verified in i18n resources.apps/meteor/client/components/message/toolbar/usePermalinkAction.ts (1)
37-37: Translation keys verified. BothAction_not_available_encrypted_contentandCopy_linkexist in i18n resource files.
MartinSchoeler
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
For some reason, when a message action is disabled, we hardcoded the
Action_not_available_encrypted_contenttooltip, this PR removes that and allows message actions to provide their own tooltipsIssue(s)
CORE-1418
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Refactor