fix: Username Overflow Fix using Ellipsis and tooltip #22570
Conversation
|
@Anshumancanrock is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
|
""" WalkthroughThe changes introduce a new React component, Estimated code review effort2 (~15 minutes) Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/modules/signup-view.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/modules/signup-view.tsx (1)
87-108: Verify the truncation logic and consider edge cases.The UrlPrefix component implementation looks mostly correct, but there are a few considerations:
- The truncation logic uses fixed percentages (60% start, 30% end) which may not work well for all URL structures
- The
maxLengthconstant of 30 seems arbitrary and might need adjustment based on UI requirements- No handling for URLs shorter than the sum of startChars + endChars + 3 (for "...")
Consider these improvements:
function UrlPrefix({ url }: { url: string }) { const maxLength = 30; // Show full URL if it's short enough if (url.length <= maxLength) { return <span className="text-muted text-sm font-medium leading-none">{url}</span>; } // For long URLs, show beginning and end with ellipsis in middle const startChars = Math.floor(maxLength * 0.6); const endChars = Math.floor(maxLength * 0.3); + + // Ensure we don't exceed the original length + const minRequiredLength = startChars + endChars + 3; // +3 for "..." + if (url.length < minRequiredLength) { + return <span className="text-muted text-sm font-medium leading-none">{url}</span>; + } + const truncated = `${url.substring(0, startChars)}...${url.substring(url.length - endChars)}`;Please test the component with various URL lengths to ensure the truncation works as expected, particularly with very short URLs that might be close to the maxLength threshold.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/modules/signup-view.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/prisma/schema.prisma:1068-1071
Timestamp: 2025-07-16T05:10:22.863Z
Learning: In PR #22304 for Cal.com private link expiration features, the `maxUsageCount` field was intentionally set to default to 1 (non-nullable) as a breaking change, making all existing private links single-use after migration. This was a deliberate design decision by alishaz-polymath.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.001Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the `currentLink.maxUsageCount ?? 1` fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
apps/web/modules/signup-view.tsx (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.001Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the `currentLink.maxUsageCount ?? 1` fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
🧬 Code Graph Analysis (1)
apps/web/modules/signup-view.tsx (2)
packages/features/ee/organizations/lib/orgDomains.ts (1)
getOrgFullOrigin(147-155)packages/lib/constants.ts (1)
URL_PROTOCOL_REGEX(166-166)
⏰ 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). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Security Check
🔇 Additional comments (4)
apps/web/modules/signup-view.tsx (4)
48-48: LGTM: Import addition is correct.The import for the Tooltip component is properly added and will be used by the new UrlPrefix component.
186-186: LGTM: String operation refactoring is correct.The change from
url.includes("?")tourl.indexOf("?") !== -1is functionally equivalent and maintains the same logic while potentially offering slight performance benefits.
255-255: LGTM: String operation refactoring is correct.The change from
redirectUrl?.includes("platform")andredirectUrl?.includes("new")toredirectUrl?.indexOf("platform") !== -1andredirectUrl?.indexOf("new") !== -1is functionally equivalent and maintains the same logic.
423-434: LGTM: Integration of UrlPrefix component is well-implemented.The integration of the new UrlPrefix component in the UsernameField's addOnLeading prop is clean and follows the existing pattern. The URL construction logic properly handles both organization slugs and regular website URLs, correctly removing the protocol using the URL_PROTOCOL_REGEX.
The component will now:
- Display truncated URLs with ellipsis for long prefixes
- Show the full URL in a tooltip on hover
- Maintain the same visual styling as before
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/16/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (07/16/25)1 label was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (07/17/25)1 label was added to this PR based on Keith Williams's automation. |
|
@cubic-dev-ai /review |
E2E results are ready! |
|
@Anshumancanrock a test seems to be failing. can you fix it? |
f69859f to
008f2e1
Compare
|
@kart1ka Fixed the test failing issue sir. And here is the video, working fine for me ! Recording.2025-07-22.024456.mp4 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/modules/signup-view.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code. Functions like .add, .diff, .isBefore, and .isAfter are slow, especially in timezone mode. Prefer .utc() for better performance. Where possible, replace with native Date and direct .valueOf() comparisons for faster execution. Recommend using native methods or Day.js .utc() consistently in hot paths like loops.
Files:
apps/web/modules/signup-view.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/prisma/schema.prisma:1068-1071
Timestamp: 2025-07-16T05:10:22.891Z
Learning: In PR #22304 for Cal.com private link expiration features, the `maxUsageCount` field was intentionally set to default to 1 (non-nullable) as a breaking change, making all existing private links single-use after migration. This was a deliberate design decision by alishaz-polymath.
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.024Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the `currentLink.maxUsageCount ?? 1` fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-21T13:54:11.770Z
Learning: Applies to frontend/**/*.{js,jsx,ts,tsx} : Always use t() for text localization in frontend code; direct text embedding should trigger a warning.
apps/web/modules/signup-view.tsx (2)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: alishaz-polymath
PR: #22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.024Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the currentLink.maxUsageCount ?? 1 fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
🧬 Code Graph Analysis (1)
apps/web/modules/signup-view.tsx (2)
packages/features/ee/organizations/lib/orgDomains.ts (1)
getOrgFullOrigin(147-155)packages/lib/constants.ts (1)
URL_PROTOCOL_REGEX(166-166)
🪛 Biome (1.9.4)
apps/web/modules/signup-view.tsx
[error] 253-253: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ 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). (1)
- GitHub Check: Security Check
🔇 Additional comments (4)
apps/web/modules/signup-view.tsx (4)
48-48: LGTM! Tooltip import added for the new component.The import addition is correct and necessary for the
UrlPrefixcomponent implementation.
87-106: Well-implemented URL truncation component with accessibility considerations.The
UrlPrefixcomponent provides a clean solution to the username field overflow issue:
- Properly truncates long URLs with ellipsis
- Uses Cal.com's Tooltip component for accessibility
- Applies consistent styling with existing components
- Uses a reasonable
maxLengthof 25 charactersThe implementation follows React best practices and provides good UX for users with long URL prefixes.
184-184: String method change is functionally equivalent.The change from
url.includes("?")tourl.indexOf("?") !== -1maintains the same logic while using a different string method. Both approaches correctly detect the presence of a query string separator.
422-431: Effective implementation of the overflow fix using the new component.The modification correctly replaces the plain string URL prefix with the new
UrlPrefixcomponent:
- Properly passes the constructed URL string as a prop
- Maintains existing logic for both organization and non-organization scenarios
- Uses
getOrgFullOriginandURL_PROTOCOL_REGEXappropriately- The URL construction logic is correct and consistent with the codebase
This change successfully addresses the PR objective of fixing username field overflow while maintaining functionality.
|
This PR is being marked as stale due to inactivity. |
What does this PR do?
This PR fixes the overflow issue for long URL prefixes in the signup username field. The prefix is now truncated with an ellipsis (...) and, on hover, a tooltip displays the full URL after a 1-second delay. The solution uses Cal.com’s Tooltip component and applies a maximum width to prevent layout breakage. This change ensures a clean, consistent user experience for both standard and premium username inputs, without affecting any other functionality.
Visual Demo (For contributors especially)
Before Changes:
After Chnages:
username-prefix-fix.1.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
UsernameFieldis rendered.addOnLeadingURL (e.g. Gitpod).Environment Setup:
Checklist