-
Notifications
You must be signed in to change notification settings - Fork 2
Profile card #4
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
Profile card #4
Conversation
Khalidnoori568
commented
Aug 20, 2025
|
@Khalidnoori568 is attempting to deploy a commit to the iclasser dev Team on Vercel. A member of the Team first needs to authorize it. |
|
test again |
|
can you resolve conflicts to see what has changed |
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.
Pull Request Overview
This PR introduces a comprehensive "Profile card" feature that includes multiple new components and updates to the existing X (Twitter) card editor. The changes add new card types including Profile, Notebook, and enhanced X cards, along with a modernized UI system using Radix UI components.
- Adds new profile card component with image placeholder and customizable content fields
- Introduces notebook-style card with checklist functionality and notebook-paper styling
- Replaces Twitter card with enhanced X card featuring dynamic metrics and improved UI
- Implements shared UI component system using Radix UI primitives and common form components
Reviewed Changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
packages/ui/src/x-preview/x-preview.tsx |
Major refactor from Twitter to X branding with new interactive metrics and timestamp functionality |
packages/ui/src/profile-preview/profile-preview.tsx |
New profile card component with gradient background and image handling |
packages/ui/src/notebookcard-preview/notebook-preview.tsx |
New notebook-style card with checklist items and paper texture styling |
apps/web/components/x-editor.tsx |
Complete replacement of Twitter editor with enhanced X card editor featuring tabs and metrics |
apps/web/components/common/index.tsx |
New shared form components using Radix UI primitives |
apps/web/app/page.tsx |
Updated navigation to include new card types and URL-based tab management |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| } catch (error) { | ||
| return num.toString(); | ||
| } | ||
| } |
Copilot
AI
Aug 22, 2025
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.
This file violates the custom coding guideline requiring new components to follow the proper structure. The X card editor should be in apps/web/components and this preview should be in packages/ui/src. While the preview is correctly placed, the editor logic (datetime handling) appears to be mixed into the preview component.
| if (num < 1_000_000) return (num / 1000).toFixed(num % 1000 === 0 ? 0 : 1) + "k"; | ||
| if (num < 1_000_000_000) return (num / 1_000_000).toFixed(num % 1_000_000 === 0 ? 0 : 1) + "m"; | ||
| if (num < 1_000_000_000_000) return (num / 1_000_000_000).toFixed(num % 1_000_000_000 === 0 ? 0 : 1) + "b"; | ||
| return (num / 1_000_000_000_000).toFixed(num % 1_000_000_000_000 === 0 ? 0 : 1) + "t"; |
Copilot
AI
Aug 22, 2025
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.
The modulo operation num % 1000 === 0 will not work correctly for determining decimal places in formatted numbers. For example, 1500 % 1000 = 500 (not 0), so it would show '1.5k' instead of '2k'. The logic should check if the division result is a whole number.
| return (num / 1_000_000_000_000).toFixed(num % 1_000_000_000_000 === 0 ? 0 : 1) + "t"; | |
| if (num < 1_000_000) return (num / 1000).toFixed((num / 1000) % 1 === 0 ? 0 : 1) + "k"; | |
| if (num < 1_000_000_000) return (num / 1_000_000).toFixed((num / 1_000_000) % 1 === 0 ? 0 : 1) + "m"; | |
| if (num < 1_000_000_000_000) return (num / 1_000_000_000).toFixed((num / 1_000_000_000) % 1 === 0 ? 0 : 1) + "b"; | |
| return (num / 1_000_000_000_000).toFixed((num / 1_000_000_000_000) % 1 === 0 ? 0 : 1) + "t"; |
| try { | ||
| if (num < 1000) return num.toString(); | ||
| if (num < 1_000_000) return (num / 1000).toFixed(num % 1000 === 0 ? 0 : 1) + "k"; | ||
| if (num < 1_000_000_000) return (num / 1_000_000).toFixed(num % 1_000_000 === 0 ? 0 : 1) + "m"; |
Copilot
AI
Aug 22, 2025
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.
Same modulo logic issue as above. num % 1_000_000 === 0 will not correctly determine when to show decimals. Should check if (num / 1_000_000) % 1 === 0 instead.
| if (num < 1_000_000_000) return (num / 1_000_000).toFixed(num % 1_000_000 === 0 ? 0 : 1) + "m"; | |
| if (num < 1_000_000_000) return (num / 1_000_000).toFixed((num / 1_000_000) % 1 === 0 ? 0 : 1) + "m"; |
| if (num < 1_000_000) return (num / 1000).toFixed(num % 1000 === 0 ? 0 : 1) + "k"; | ||
| if (num < 1_000_000_000) return (num / 1_000_000).toFixed(num % 1_000_000 === 0 ? 0 : 1) + "m"; | ||
| if (num < 1_000_000_000_000) return (num / 1_000_000_000).toFixed(num % 1_000_000_000 === 0 ? 0 : 1) + "b"; | ||
| return (num / 1_000_000_000_000).toFixed(num % 1_000_000_000_000 === 0 ? 0 : 1) + "t"; |
Copilot
AI
Aug 22, 2025
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.
Same modulo logic issue as above. The condition num % 1_000_000_000 === 0 will not work correctly for determining decimal places in the formatted output.
| return (num / 1_000_000_000_000).toFixed(num % 1_000_000_000_000 === 0 ? 0 : 1) + "t"; | |
| if (num < 1_000_000) return (num / 1000).toFixed(Number.isInteger(num / 1000) ? 0 : 1) + "k"; | |
| if (num < 1_000_000_000) return (num / 1_000_000).toFixed(Number.isInteger(num / 1_000_000) ? 0 : 1) + "m"; | |
| if (num < 1_000_000_000_000) return (num / 1_000_000_000).toFixed(Number.isInteger(num / 1_000_000_000) ? 0 : 1) + "b"; | |
| return (num / 1_000_000_000_000).toFixed(Number.isInteger(num / 1_000_000_000_000) ? 0 : 1) + "t"; |
| if (num < 1_000_000) return (num / 1000).toFixed(num % 1000 === 0 ? 0 : 1) + "k"; | ||
| if (num < 1_000_000_000) return (num / 1_000_000).toFixed(num % 1_000_000 === 0 ? 0 : 1) + "m"; | ||
| if (num < 1_000_000_000_000) return (num / 1_000_000_000).toFixed(num % 1_000_000_000 === 0 ? 0 : 1) + "b"; | ||
| return (num / 1_000_000_000_000).toFixed(num % 1_000_000_000_000 === 0 ? 0 : 1) + "t"; |
Copilot
AI
Aug 22, 2025
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.
Same modulo logic issue as above. The condition for determining decimal places is incorrect and will not produce the expected formatting behavior.
| return (num / 1_000_000_000_000).toFixed(num % 1_000_000_000_000 === 0 ? 0 : 1) + "t"; | |
| if (num < 1_000_000) return (num / 1000).toFixed(Number.isInteger(num / 1000) ? 0 : 1) + "k"; | |
| if (num < 1_000_000_000) return (num / 1_000_000).toFixed(Number.isInteger(num / 1_000_000) ? 0 : 1) + "m"; | |
| if (num < 1_000_000_000_000) return (num / 1_000_000_000).toFixed(Number.isInteger(num / 1_000_000_000) ? 0 : 1) + "b"; | |
| return (num / 1_000_000_000_000).toFixed(Number.isInteger(num / 1_000_000_000_000) ? 0 : 1) + "t"; |
| max={max} | ||
| name={keyName} | ||
| value={[value]} | ||
| onValueChange={(val) => setStateValue(keyName, val)} |
Copilot
AI
Aug 22, 2025
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.
The Slider component returns an array of values, but the function is passing the entire array to setStateValue. This should extract the first value: onValueChange={(val) => setStateValue(keyName, val[0])}
| onValueChange={(val) => setStateValue(keyName, val)} | |
| onValueChange={(val) => setStateValue(keyName, val[0])} |
|
|
||
| export default function Home() { | ||
| return ( | ||
| <Suspense fallback={<div>Loading...</div>}> |
Copilot
AI
Aug 22, 2025
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.
According to the coding guidelines, new components should have tabs added to apps/web/app/page.tsx. While tabs were added, the structure doesn't clearly follow the required pattern of having separate tabs for each new component (Profile card, Notebook card, and X card).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|