fix: replace line-clamp with custom logic in app card#23660
fix: replace line-clamp with custom logic in app card#23660bandhan-majumder wants to merge 7 commits intocalcom:mainfrom
Conversation
|
@bandhan-majumder is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds an internal helper truncateText(text, maxLength = 130) inside packages/features/apps/components/AppCard.tsx that truncates long descriptions by character count, preserving word boundaries (uses last whitespace before the limit or the substring if none) and appends an ellipsis when truncated. The component now renders {truncateText(app.description)} for the description and replaces the prior CSS 3-line clamp usage with a numeric WebkitLineClamp: 3. No exported/public interfaces were changed. Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNone found. ✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
dosubot
bot
added
the
🧹 Improvements
![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 (3)
packages/features/apps/components/AppCard.tsx (3)
89-95: Make truncation grapheme-safe and avoid mid-codepoint splits.Substring on UTF-16 can split emoji/combined characters and some CJK. Also, keep word-boundary behavior. Minor perf: define outside component or memoize, but not blocking.
Consider this drop-in replacement:
- const truncateText = (text: string, maxLength = 170): string => { - if (!text || text.length <= maxLength) return text; - const truncated = text.substring(0, maxLength); - const lastSpace = truncated.lastIndexOf(" "); - return `${lastSpace > 0 ? text.substring(0, lastSpace) : truncated}...`; - }; + const truncateText = (text: string, maxLength = 170): string => { + if (!text) return ""; + const clean = text.trim(); + if (clean.length <= maxLength) return clean; + const cps = Array.from(clean); // grapheme-safe slice + const slice = cps.slice(0, maxLength + 1).join(""); + const lastSpace = slice.lastIndexOf(" "); + const head = lastSpace > 0 ? slice.slice(0, lastSpace) : cps.slice(0, maxLength).join(""); + return `${head}…`; + };Please verify with samples: long CJK text, emoji + ZWJ sequences, and no-space strings (e.g., 200-char “A”). Want me to add a quick unit test for these cases?
127-127: Small UX boost: show full description on hover (non-blocking).Adding a title preserves discoverability after truncation; dir helps RTL.
- <p className="text-default mt-2 flex-grow text-sm">{truncateText(app.description)}</p> + <p className="text-default mt-2 flex-grow text-sm" title={app.description} dir="auto"> + {truncateText(app.description)} + </p>
188-188: Localize literal “Template”.Per guidelines, wrap user-facing strings with t().
- <span className="bg-error rounded-md px-2 py-1 text-sm font-normal text-red-800">Template</span> + <span className="bg-error rounded-md px-2 py-1 text-sm font-normal text-red-800">{t("template")}</span>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/apps/components/AppCard.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/apps/components/AppCard.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/apps/components/AppCard.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/apps/components/AppCard.tsx
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
kart1ka
left a comment
kart1ka
left a comment
There was a problem hiding this comment.
This does not work properly and causes other issues with the layout such as messing with the position of the Details and Install buttons. See the image attached.
|
sorry for that. fixed now |
kart1ka
left a comment
kart1ka
left a comment
There was a problem hiding this comment.
does not work for every app card. see the image attached.
|
@kart1ka |
|
This PR is being marked as stale due to inactivity. |
|
@dhairyashiil Here is the video in both desktop and mobile view. It works fine imo.. please check Recording.2025-12-11.195333.1.1.1.mp4 |
|
Fixed by this:- #26722 |
4 participants
What does this PR do?
Replaces line-clamp property with custom truncate logic as line-clamp is not supported properly by widely used browsers. More about it: https://developer.mozilla.org/en-US/docs/Web/CSS/line-clamp
line-clampproperty does not truncate long texts properly in app card description #23659 (GitHub issue number)Image Demo (if applicable):
first
second
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist