feat: add new UI components including Navbar, Hero, BentoGrid, and Ka…#64
feat: add new UI components including Navbar, Hero, BentoGrid, and Ka…#64
Conversation
…ryaCard, and integrate them into the application structure.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @Ardelyo, 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! Pull request ini berfokus pada peningkatan performa dan pengalaman pengguna aplikasi dengan mengoptimalkan pemuatan sumber daya dan rendering komponen UI. Perubahan utama meliputi implementasi lazy loading untuk halaman otentikasi, optimasi gambar untuk mencegah CLS, peningkatan Largest Contentful Paint (LCP) pada elemen hero, serta penambahan properti 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
Pull request ini memperkenalkan beberapa komponen UI baru dan melakukan serangkaian optimasi performa yang signifikan. Perubahan seperti penerapan lazy loading untuk halaman otentikasi, penambahan atribut untuk optimasi LCP, serta perbaikan struktur HTML yang tidak valid adalah langkah-langkah yang sangat baik untuk meningkatkan kualitas dan kecepatan aplikasi.
Secara keseluruhan, kode yang diajukan berkualitas tinggi. Saya hanya memberikan beberapa saran untuk penyempurnaan lebih lanjut terkait praktik terbaik untuk menjaga type safety di TypeScript dan penggunaan properti CSS will-change yang lebih optimal untuk performa.
| // @ts-ignore - fetchpriority is valid in Chromium | ||
| fetchpriority="high" |
There was a problem hiding this comment.
Penggunaan @ts-ignore sebaiknya dihindari untuk menjaga keamanan tipe (type safety). Sebagai alternatif yang lebih baik dan dapat digunakan kembali di seluruh proyek, Anda dapat mendeklarasikan properti fetchpriority dalam sebuah file deklarasi TypeScript (misalnya, react.d.ts).
Contoh isi file react.d.ts:
declare module 'react' {
interface HTMLAttributes<T> extends AriaAttributes, DOMAttributes<T> {
fetchpriority?: 'high' | 'low' | 'auto';
}
}Ini akan memberikan IntelliSense dan pemeriksaan tipe yang benar tanpa harus menonaktifkan pemeriksaan TypeScript.
| <div className="relative w-full aspect-[4/5] md:aspect-[3/4]"> | ||
| <div className="w-full h-full overflow-hidden"> | ||
| <div className="relative w-full aspect-[4/5] md:aspect-[3/4] overflow-hidden bg-neutral-900"> | ||
| <div className="w-full h-full overflow-hidden will-change-transform"> |
There was a problem hiding this comment.
Penggunaan will-change-transform di sini mungkin tidak memberikan manfaat performa yang diharapkan dan berpotensi menyebabkan penggunaan memori yang tidak perlu. Properti will-change sebaiknya diterapkan langsung pada elemen yang akan dianimasikan.
Dalam kasus ini, animasi transform (seperti scale pada gambar atau y pada kartu) terjadi pada elemen lain (elemen motion.div induk atau img di dalamnya). Sebaiknya hapus will-change-transform dari div ini. Jika optimasi spesifik diperlukan, terapkan langsung pada elemen gambar yang memiliki transisi scale.
| <div className="w-full h-full overflow-hidden will-change-transform"> | |
| <div className="w-full h-full overflow-hidden"> |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Missing clearTimeout on component unmount causes potential memory leak
The scroll handler in Navbar.tsx doesn't clear the pending timeout when the component unmounts, which can cause a memory leak and React state update warnings.
Click to expand
Issue
In the scroll effect at components/Navbar.tsx:37-48, a timeoutId is set but never cleared on cleanup:
useEffect(() => {
let timeoutId: NodeJS.Timeout;
const handleScroll = () => {
if (timeoutId) return;
timeoutId = setTimeout(() => {
setIsScrolled(window.scrollY > 50);
timeoutId = undefined!;
}, 100);
};
window.addEventListener('scroll', handleScroll);
return () => window.removeEventListener('scroll', handleScroll);
}, []);Actual vs Expected
- Actual: If user scrolls then navigates away before the 100ms timeout fires,
setIsScrolledwill be called on an unmounted component, causing a React warning and potential memory leak. - Expected: The cleanup function should also call
clearTimeout(timeoutId)to cancel any pending timeout.
Impact
This can cause "Can't perform a React state update on an unmounted component" warnings and minor memory leaks during rapid navigation.
(Refers to lines 37-48)
Recommendation: Add if (timeoutId) clearTimeout(timeoutId); to the cleanup function: return () => { if (timeoutId) clearTimeout(timeoutId); window.removeEventListener('scroll', handleScroll); };
Was this helpful? React with 👍 or 👎 to provide feedback.
| style={{ | ||
| background: `radial-gradient(circle 350px at ${springX}px ${springY}px, rgba(255,255,240,0.4) 0%, rgba(0,0,0,0.95) 100%)` | ||
| }} |
There was a problem hiding this comment.
🔴 MotionValue objects used in template literal produce invalid CSS
The HangingLamp component's spotlight effect is broken because springX and springY (which are MotionValue objects from framer-motion) are interpolated directly into template literal strings for the CSS background property.
Click to expand
How it happens
At pages/divisions/Graphics.tsx:91-93 and pages/divisions/Graphics.tsx:98-100, the code does:
style={{
background: `radial-gradient(circle 350px at ${springX}px ${springY}px, ...)`
}}MotionValues are objects, not primitives. When used in a template literal, JavaScript calls .toString() on them, which returns "[object Object]" instead of the numeric value.
Actual vs Expected
- Actual: The CSS becomes
radial-gradient(circle 350px at [object Object]px [object Object]px, ...)which is invalid CSS, causing the flashlight/spotlight effect to not work at all. - Expected: The gradient should follow the mouse cursor position dynamically.
Impact
The entire interactive "hanging lamp" flashlight effect on the Graphics division page is completely non-functional.
Recommendation: Use framer-motion's useTransform to create derived values, or use the useMotionTemplate utility from framer-motion, e.g.:
const background = useMotionTemplate`radial-gradient(circle 350px at ${springX}px ${springY}px, rgba(255,255,240,0.4) 0%, rgba(0,0,0,0.95) 100%)`;Then use style={{ background }} in the motion.div.
Was this helpful? React with 👍 or 👎 to provide feedback.
| <div className="flex-shrink-0 relative group"> | ||
| <div className="w-48 h-48 md:w-64 md:h-64 relative transform rotate-3"> | ||
| <div className="absolute inset-0 bg-white shadow-2xl transform transition-transform group-hover:rotate-0" /> | ||
| <img src="/logo-oc-desain.jpg" alt="OC Design Logo" className="relative z-10 w-full h-full object-cover p-2 gray-scale group-hover:grayscale-0 transition-all duration-500" /> |
There was a problem hiding this comment.
🟡 Invalid Tailwind CSS class 'gray-scale' should be 'grayscale'
The CSS class gray-scale is not a valid Tailwind CSS utility class. The correct class name is grayscale (without the hyphen).
Click to expand
Location
pages/divisions/Graphics.tsx:210:
<img ... className="... gray-scale group-hover:grayscale-0 ..." />Actual vs Expected
- Actual: The
gray-scaleclass does nothing because it's not recognized by Tailwind. The image won't have a grayscale filter applied initially. - Expected: The image should be grayscale by default and become colored on hover.
Impact
The visual design intent (grayscale-to-color hover effect on the logo) doesn't work correctly - the image will always be in color.
Recommendation: Change gray-scale to grayscale:
className="relative z-10 w-full h-full object-cover p-2 grayscale group-hover:grayscale-0 transition-all duration-500"
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
…ryaCard, and integrate them into the application structure.
📋 Deskripsi
Jelaskan perubahan yang kamu buat secara singkat.
🔗 Issue Terkait
Fixes #(nomor issue)
🔄 Tipe Perubahan
📸 Screenshot (jika UI berubah)
✅ Checklist
📝 Catatan untuk Reviewer
Tambahkan catatan atau pertanyaan untuk reviewer di sini.