Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @evanwang9x, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the application's content and user interface by introducing comprehensive pages for couple types. It lays the groundwork for detailed type descriptions, interactive sharing, and improved navigation, while also updating the styling with new fonts and color schemes. The changes enhance the user experience by providing more in-depth information and better accessibility to different couple types. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds new pages and components for displaying couple types, including a new font and button styles. The changes are a good step forward, but I've identified a few areas for improvement. My review focuses on a critical correctness issue with page props, several instances of significant code duplication that affect maintainability, and some data integrity problems in your constants. The feedback includes specific suggestions to resolve these issues, refactor duplicated code, and improve the overall quality of the code.
| params: Promise<{ | ||
| typeCode: string; | ||
| }>; | ||
| } | ||
|
|
||
| export default async function TypePage({ params }: TypePageProps) { | ||
| const { typeCode } = await params; |
There was a problem hiding this comment.
The params prop in Next.js App Router server components is an object, not a Promise. The current typing and usage with await will cause a runtime error. You should update the TypePageProps interface and remove the await when destructuring typeCode.
interface TypePageProps {
params: {
typeCode: string;
};
}
export default async function TypePage({ params }: TypePageProps) {
const { typeCode } = params;
| dateIdeas: [ | ||
| "Cozy coffee shop study dates", | ||
| "Museum strolls with shared playlists", | ||
| "Cozy coffee shop study dates", | ||
| "Museum strolls with shared playlists", | ||
| "Cozy coffee shop study dates", | ||
| ], | ||
| conflictResolutionText: | ||
| "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.", | ||
| loveLanguages: [ | ||
| "Quality time", | ||
| "Words of affirmation", | ||
| "Thoughtful gestures that say 'I remembered'", | ||
| "Museum strolls with shared playlists", | ||
| "Cozy coffee shop study dates", | ||
| ], |
There was a problem hiding this comment.
There appear to be some copy-paste errors in the data for the OLIN_AND_URIS type:
- The
dateIdeasarray contains duplicate entries. - The
loveLanguagesarray contains values that look like date ideas ("Museum strolls with shared playlists", "Cozy coffee shop study dates") instead of actual love languages.
Please review and correct this data to ensure its accuracy.
dateIdeas: [
"Cozy coffee shop study dates",
"Museum strolls with shared playlists"
],
conflictResolutionText:
"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.",
loveLanguages: [
"Quality time",
"Words of affirmation",
"Thoughtful gestures that say 'I remembered'"
],| <div className="space-y-3.5"> | ||
| {/* Planned vs Spontaneous */} | ||
| <div> | ||
| <div className="flex justify-between mb-1"> | ||
| <span className="text-base font-semibold text-blue-900">Planned</span> | ||
| <span className="text-base font-semibold text-pink-500">Spontaneous</span> | ||
| </div> | ||
| <div className="relative h-10 bg-pink-100 rounded-full overflow-hidden" style={{ boxShadow: '3px 3px 0px rgb(241, 168, 176)' }}> | ||
| <div | ||
| className="absolute left-0 top-0 h-full bg-blue-900 flex items-center justify-start px-3" | ||
| style={{ width: `${coupleType.dimensions?.planned || 50}%` }} | ||
| > | ||
| <span className="text-white font-bold text-base">{coupleType.dimensions?.planned || 50}%</span> | ||
| </div> | ||
| <div | ||
| className="absolute right-0 top-0 h-full flex items-center justify-end px-3" | ||
| style={{ width: `${100 - (coupleType.dimensions?.planned || 50)}%` }} | ||
| > | ||
| <span className="text-pink-500 font-bold text-base">{100 - (coupleType.dimensions?.planned || 50)}%</span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Physical vs Emotional */} | ||
| <div> | ||
| <div className="flex justify-between mb-1"> | ||
| <span className="text-base font-semibold text-pink-500">Physical</span> | ||
| <span className="text-base font-semibold text-blue-900">Emotional</span> | ||
| </div> | ||
| <div className="relative h-10 bg-blue-100 rounded-full overflow-hidden" style={{ boxShadow: '3px 3px 0px rgb(241, 168, 176)' }}> | ||
| <div | ||
| className="absolute left-0 top-0 h-full bg-pink-100 flex items-center justify-start px-3" | ||
| style={{ width: `${coupleType.dimensions?.physical || 50}%` }} | ||
| > | ||
| <span className="text-pink-500 font-bold text-base">{coupleType.dimensions?.physical || 50}%</span> | ||
| </div> | ||
| <div | ||
| className="absolute right-0 top-0 h-full bg-blue-900 flex items-center justify-end px-3" | ||
| style={{ width: `${100 - (coupleType.dimensions?.physical || 50)}%` }} | ||
| > | ||
| <span className="text-white font-bold text-base">{100 - (coupleType.dimensions?.physical || 50)}%</span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Reflective vs Active */} | ||
| <div> | ||
| <div className="flex justify-between mb-1"> | ||
| <span className="text-base font-semibold text-blue-900">Reflective</span> | ||
| <span className="text-base font-semibold text-pink-500">Active</span> | ||
| </div> | ||
| <div className="relative h-10 bg-pink-100 rounded-full overflow-hidden" style={{ boxShadow: '3px 3px 0px rgb(241, 168, 176)' }}> | ||
| <div | ||
| className="absolute left-0 top-0 h-full bg-blue-900 flex items-center justify-start px-3" | ||
| style={{ width: `${coupleType.dimensions?.reflective || 50}%` }} | ||
| > | ||
| <span className="text-white font-bold text-base">{coupleType.dimensions?.reflective || 50}%</span> | ||
| </div> | ||
| <div | ||
| className="absolute right-0 top-0 h-full flex items-center justify-end px-3" | ||
| style={{ width: `${100 - (coupleType.dimensions?.reflective || 50)}%` }} | ||
| > | ||
| <span className="text-pink-500 font-bold text-base">{100 - (coupleType.dimensions?.reflective || 50)}%</span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
The code for rendering the three personality bars (Planned vs Spontaneous, Physical vs Emotional, Reflective vs Active) is very repetitive. This could be extracted into a reusable component to improve maintainability and readability. Consider creating a PersonalityBar component that accepts props for the labels, colors, and percentage value.
| <button className="hover:scale-110 transition-transform flex-shrink-0"> | ||
| <Image src="/facebookIcon.png" alt="Share on Facebook" width={32} height={32} className="w-6 h-6" /> | ||
| </button> | ||
| <button className="hover:scale-110 transition-transform flex-shrink-0"> | ||
| <Image src="/twitterIcon.png" alt="Share on Twitter" width={32} height={32} className="w-6 h-6" /> | ||
| </button> | ||
| <button className="hover:scale-110 transition-transform flex-shrink-0"> | ||
| <Image src="/instagramIcon.png" alt="Share on Instagram" width={32} height={32} className="w-6 h-6" /> | ||
| </button> | ||
| <button className="hover:scale-110 transition-transform flex-shrink-0"> | ||
| <Image src="/tikTokIcon.png" alt="Share on TikTok" width={32} height={32} className="w-6 h-6" /> | ||
| </button> | ||
| <button className="hover:scale-110 transition-transform flex-shrink-0"> | ||
| <Image src="/wechatIcon.png" alt="Share on WeChat" width={32} height={32} className="w-6 h-6" /> | ||
| </button> | ||
| <button className="hover:scale-110 transition-transform flex-shrink-0"> | ||
| <Image src="/downloadIcon.png" alt="Download" width={32} height={32} className="w-6 h-6" /> | ||
| </button> |
There was a problem hiding this comment.
These buttons for social media sharing currently have no functionality. For them to work as expected, they should either be anchor tags (<a>) pointing to the respective sharing URLs, or have onClick handlers to trigger sharing functionality (e.g., using the Web Share API or opening a popup). Also, it's a good practice to add type="button" to prevent them from submitting a form if they are ever placed inside one.
| import { Navbar } from "@/components/navbar"; | ||
|
|
||
| export default function AdventurousPlanners() { | ||
| return ( | ||
| <div> | ||
| <h1>Hello World - The Adventure Planners</h1> | ||
| </div> | ||
| ); | ||
| } No newline at end of file |
There was a problem hiding this comment.
This file appears to be obsolete. The couple type ADVENTUROUS_PLANNERS was renamed to OLIN_AND_URIS in coupleTypes.ts. This component doesn't seem to be used anywhere in the app and could likely be removed to clean up the codebase. It also unnecessarily imports Navbar, which is already handled by the root layout.
| secondary: "bg-secondary text-secondary-foreground shadow-xs hover:bg-secondary/80", | ||
| ghost: "hover:bg-accent hover:text-accent-foreground dark:hover:bg-accent/50", | ||
| link: "text-primary underline-offset-4 hover:underline", | ||
| pressed: "!rounded-full bg-white text-pmred-500 border-4 border-pmblue-500 font-bold shadow-[6px_6px_0px_0px_rgba(36,67,141,1)] hover:translate-x-[4px] hover:translate-y-[4px] hover:shadow-[2px_2px_0px_0px_rgba(36,67,141,1)] active:translate-x-[6px] active:translate-y-[6px] active:shadow-none", |
There was a problem hiding this comment.
The new pressed variant for the button uses a hardcoded RGBA color value for its shadow. A CSS variable --color-pmblue-500 was added for this color in globals.css. Using the variable (var(--color-pmblue-500)) will improve maintainability and ensure consistency with your theme.
| pressed: "!rounded-full bg-white text-pmred-500 border-4 border-pmblue-500 font-bold shadow-[6px_6px_0px_0px_rgba(36,67,141,1)] hover:translate-x-[4px] hover:translate-y-[4px] hover:shadow-[2px_2px_0px_0px_rgba(36,67,141,1)] active:translate-x-[6px] active:translate-y-[6px] active:shadow-none", | |
| pressed: "!rounded-full bg-white text-pmred-500 border-4 border-pmblue-500 font-bold shadow-[6px_6px_0px_0px_var(--color-pmblue-500)] hover:translate-x-[4px] hover:translate-y-[4px] hover:shadow-[2px_2px_0px_0px_var(--color-pmblue-500)] active:translate-x-[6px] active:translate-y-[6px] active:shadow-none", |
1 participant
-Need to refactor code to be less repetitive
-Still need to figure out how to tilt the wavy dividers.
-Need to fix first section display on mobile view.
-Need to fix positioning of the hearts.
-Need to fix the color schematics.
-Need to pull footer from main website
-Need to add 15 other couple types
-Need to add 2 more hearts to the second section
Screen.Recording.2025-11-13.at.9.29.33.PM.1.mov