-
Notifications
You must be signed in to change notification settings - Fork 13k
feat: Change CTA position in FingerprintChangeModal
#37659
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 |
🦋 Changeset detectedLatest commit: ad97ea3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR inverts button positions and labels in the Unique ID change detected modal to redirect user attention from "New workspace" to "Configuration update". The confirm button now displays "Configuration_update" with CTA styling, while the cancel button shows "New_workspace". Handler logic is synchronized to match the reversed button behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 #37659 +/- ##
===========================================
- Coverage 68.81% 68.81% -0.01%
===========================================
Files 3361 3361
Lines 114260 114204 -56
Branches 20619 20619
===========================================
- Hits 78629 78586 -43
+ Misses 33533 33520 -13
Partials 2098 2098
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/fuzzy-plants-hammer.md (1)
5-5: Reword changeset description for concision and clarityYou can tighten the sentence and make the intent clearer by dropping “in order to” and clarifying the emphasized action.
-Changes the position of the buttons in Unique ID change detected modal in order to highlight configuration update instead of new workspace +Changes the position of the buttons in the Unique ID change detected modal to highlight the configuration update action instead of creating a new workspace.
📜 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 (3)
.changeset/fuzzy-plants-hammer.md(1 hunks)apps/meteor/client/components/FingerprintChangeModal.tsx(1 hunks)apps/meteor/client/views/root/hooks/loggedIn/useFingerprintChange.tsx(1 hunks)
🧰 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/views/root/hooks/loggedIn/useFingerprintChange.tsxapps/meteor/client/components/FingerprintChangeModal.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
.changeset/fuzzy-plants-hammer.md
🪛 LanguageTool
.changeset/fuzzy-plants-hammer.md
[style] ~5-~5: Consider a more concise word here.
Context: ...tons in Unique ID change detected modal in order to highlight configuration update instead ...
(IN_ORDER_TO_PREMIUM)
⏰ 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). (10)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
🔇 Additional comments (2)
apps/meteor/client/views/root/hooks/loggedIn/useFingerprintChange.tsx (1)
77-78: Confirm/cancel handlers now align with inverted CTA semantics
onConfirmsettingnewWorkspace: falseandonCancelsettingnewWorkspace: truecorrectly inverts the paths so the primary action corresponds to a configuration update while still allowing the alternative “new workspace” flow via the secondary button, matching thefingerPrintMutation(newWorkspace ? 'new-workspace' : 'updated-configuration')logic.apps/meteor/client/components/FingerprintChangeModal.tsx (1)
24-25: Swapped button labels correctly emphasize configuration update as CTAUsing
confirmText={t('Configuration_update')}andcancelText={t('New_workspace')}matches the product goal of making configuration update the primary CTA and is consistent with the updateduseFingerprintChangehandlers.
Proposed changes (including videos or screenshots)
The
FingerprintChangeModalhas pushing people to click on “New workspace” by accident and that creates demand for our support team to update their register, while that doesn’t happen, premium customers stay stuck in the free version, right after they click on new workspace.This pull request changes the position of the "configuration update" and "new workspace" buttons in
FingerprintChangeModalin order to highlight configuration update instead of new workspace to avoid this mistakebefore
after
Issue(s)
Steps to test or reproduce
Further comments
CORE-1541
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.