-
Notifications
You must be signed in to change notification settings - Fork 0
Feature#385 메인페이지 youtube embed 영상 삽입 #386
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaced the homepage HeroSection with a new YoutubeHeroSection that embeds a YouTube video. Added a new hook to detect font load state for coordinating GSAP animations. Introduced corresponding SCSS module for the new hero and removed an empty media query from the old HeroSection styles. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant NextPage as pages/index.tsx
participant YTHero as YoutubeHeroSection
participant Hook as useFontLoadState
participant Fonts as document.fonts
participant GSAP as GSAP Timeline
participant Router as Next Router
User->>NextPage: Load "/"
NextPage->>YTHero: Render component
YTHero->>Hook: useFontLoadState()
alt fonts API available
Hook->>Fonts: await fonts.ready
Fonts-->>Hook: resolved
else fallback
Hook-->>YTHero: timeout (~100ms)
end
Hook-->>YTHero: fontLoaded = true
YTHero->>GSAP: Play intro animations (title/content)
User->>YTHero: Click "지금 바로 상담받기" CTA
YTHero->>Router: navigate to "/apply"
Router-->>User: Apply page rendered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(None) Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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 |
📝 추가 및 변경된 파일총 5개 파일 변경 |
📚 Storybook이 Chromatic에 배포되었습니다!
|
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.
Actionable comments posted: 5
🧹 Nitpick comments (4)
mosu-app/src/shared/hooks/useFontLoadState.ts (1)
6-14: Avoid setState after unmount; add cleanup and timeout handleGuard against unmount and clear the fallback timeout to prevent memory leaks and React warnings.
- useEffect(() => { - if (document.fonts && document.fonts.ready) { - document.fonts.ready.then(() => { - setFontLoaded(true); - }); - } else { - setTimeout(() => setFontLoaded(true), 100); - } - }, []); + useEffect(() => { + let mounted = true; + let timeoutId: number | null = null; + const markLoaded = () => mounted && setFontLoaded(true); + + if (document.fonts && document.fonts.ready) { + document.fonts.ready.then(markLoaded); + } else { + timeoutId = window.setTimeout(markLoaded, 150); + } + return () => { + mounted = false; + if (timeoutId !== null) clearTimeout(timeoutId); + }; + }, []);mosu-app/src/widgets/home/YoutubeHeroSection.module.scss (1)
16-17: Provide fallback foroverflow: clipOlder browsers may not support
clip. Layer ahiddenfallback.- overflow: clip; + overflow: hidden; /* fallback */ + overflow: clip;mosu-app/src/pages/index.tsx (1)
9-9: Remove commented-out codeDrop the commented import and component usage to keep the page clean.
-// import { HeroSection } from "@/widgets/home/HeroSection";-{/* <HeroSection /> */}Also applies to: 27-28
mosu-app/src/widgets/home/YoutubeHeroSection.tsx (1)
15-15: Remove unused ref
registerButtonRefis no longer used after converting the CTA to a Link-only anchor.- const registerButtonRef = useRef<HTMLButtonElement>(null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
mosu-app/src/pages/index.tsx(2 hunks)mosu-app/src/shared/hooks/useFontLoadState.ts(1 hunks)mosu-app/src/widgets/home/HeroSection.module.scss(0 hunks)mosu-app/src/widgets/home/YoutubeHeroSection.module.scss(1 hunks)mosu-app/src/widgets/home/YoutubeHeroSection.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- mosu-app/src/widgets/home/HeroSection.module.scss
🧰 Additional context used
🧬 Code graph analysis (2)
mosu-app/src/pages/index.tsx (1)
mosu-app/src/widgets/home/YoutubeHeroSection.tsx (1)
YoutubeHeroSection(12-56)
mosu-app/src/widgets/home/YoutubeHeroSection.tsx (1)
mosu-app/src/shared/hooks/useFontLoadState.ts (1)
useFontLoadState(3-17)
⏰ 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: Run DangerJS
- GitHub Check: Run Unit & Integration Tests
🔇 Additional comments (4)
mosu-app/src/shared/hooks/useFontLoadState.ts (1)
3-17: Hook looks good overallClient-only access via useEffect and a sane fallback path.
mosu-app/src/widgets/home/YoutubeHeroSection.module.scss (1)
9-12: Verify header/banner heights or derive from CSS varsHard-coding 100px/70px may drift from real header/banner sizes across breakpoints.
If you already expose header/banner heights as CSS variables, consider:
- height: calc(100vh - 100px - 70px); + /* Example if --header-h and --banner-h are available */ + height: calc(100vh - var(--header-h, 100px) - var(--banner-h, 70px));mosu-app/src/pages/index.tsx (1)
16-16: New hero import looks goodHomepage now clearly sources the YouTube hero.
mosu-app/src/widgets/home/YoutubeHeroSection.tsx (1)
25-55: Overall structure and animation trigger look goodTitle/content opacity gating on font load is sensible; timeline usage is clear.
| button { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 0.75rem; | ||
|
|
||
| width: fit-content; | ||
|
|
||
| margin: 0.5rem auto; | ||
| border-radius: 9999px; | ||
| padding: 0.5rem 1rem; | ||
|
|
||
| background-color: #fff; | ||
| transition: background-color 1s; | ||
|
|
||
| font-size: 1rem; | ||
|
|
||
| @media (max-width: 1023px) { | ||
| padding: 0.5rem 1rem; | ||
| font-size: 1rem; | ||
| } | ||
| @media (max-width: 768px) { | ||
| padding: 0.5rem 1rem; | ||
| font-size: 0.875rem; | ||
| } | ||
|
|
||
| &:hover { | ||
| cursor: pointer; | ||
| background-color: #f3f3f3; | ||
| } | ||
|
|
||
| span:first-child { | ||
| color: #000; | ||
| } | ||
| span:last-child { | ||
| aspect-ratio: 1 / 1; | ||
| height: fit-content; | ||
|
|
||
| padding: 0.25rem; | ||
| border-radius: 9999px; | ||
|
|
||
| background-color: #000; | ||
| } | ||
| } |
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.
🛠️ Refactor suggestion
Align styles with Link-as-anchor (not button) and improve a11y focus
The CTA is an anchor via Next.js Link; style the anchor instead of a button and add focus-visible.
- button {
+ a {
display: flex;
align-items: center;
gap: 0.75rem;
width: fit-content;
margin: 0.5rem auto;
border-radius: 9999px;
padding: 0.5rem 1rem;
background-color: #fff;
transition: background-color 1s;
font-size: 1rem;
+ text-decoration: none; /* anchor reset */
@media (max-width: 1023px) {
padding: 0.5rem 1rem;
font-size: 1rem;
}
@media (max-width: 768px) {
padding: 0.5rem 1rem;
font-size: 0.875rem;
}
&:hover {
cursor: pointer;
background-color: #f3f3f3;
}
+ &:focus-visible {
+ outline: 2px solid #fff;
+ outline-offset: 2px;
+ }
span:first-child {
color: #000;
}
span:last-child {
aspect-ratio: 1 / 1;
height: fit-content;
padding: 0.25rem;
border-radius: 9999px;
background-color: #000;
+ color: #fff; /* ensure chevron is visible on black */
}
- }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| button { | |
| display: flex; | |
| align-items: center; | |
| gap: 0.75rem; | |
| width: fit-content; | |
| margin: 0.5rem auto; | |
| border-radius: 9999px; | |
| padding: 0.5rem 1rem; | |
| background-color: #fff; | |
| transition: background-color 1s; | |
| font-size: 1rem; | |
| @media (max-width: 1023px) { | |
| padding: 0.5rem 1rem; | |
| font-size: 1rem; | |
| } | |
| @media (max-width: 768px) { | |
| padding: 0.5rem 1rem; | |
| font-size: 0.875rem; | |
| } | |
| &:hover { | |
| cursor: pointer; | |
| background-color: #f3f3f3; | |
| } | |
| span:first-child { | |
| color: #000; | |
| } | |
| span:last-child { | |
| aspect-ratio: 1 / 1; | |
| height: fit-content; | |
| padding: 0.25rem; | |
| border-radius: 9999px; | |
| background-color: #000; | |
| } | |
| } | |
| a { | |
| display: flex; | |
| align-items: center; | |
| gap: 0.75rem; | |
| width: fit-content; | |
| margin: 0.5rem auto; | |
| border-radius: 9999px; | |
| padding: 0.5rem 1rem; | |
| background-color: #fff; | |
| transition: background-color 1s; | |
| font-size: 1rem; | |
| text-decoration: none; /* anchor reset */ | |
| @media (max-width: 1023px) { | |
| padding: 0.5rem 1rem; | |
| font-size: 1rem; | |
| } | |
| @media (max-width: 768px) { | |
| padding: 0.5rem 1rem; | |
| font-size: 0.875rem; | |
| } | |
| &:hover { | |
| cursor: pointer; | |
| background-color: #f3f3f3; | |
| } | |
| &:focus-visible { | |
| outline: 2px solid #fff; | |
| outline-offset: 2px; | |
| } | |
| span:first-child { | |
| color: #000; | |
| } | |
| span:last-child { | |
| aspect-ratio: 1 / 1; | |
| height: fit-content; | |
| padding: 0.25rem; | |
| border-radius: 9999px; | |
| background-color: #000; | |
| color: #fff; /* ensure chevron is visible on black */ | |
| } | |
| } |
| useGSAP(() => { | ||
| if (!fontLoaded) return; | ||
|
|
||
| const timeline = gsap.timeline(); | ||
| timeline.fromTo(titleRef.current, { opacity: 0, y: 50 }, { opacity: 1, y: 0, duration: 2, ease: "expo.out" }); | ||
| }, [fontLoaded]); |
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.
💡 Verification agent
🧩 Analysis chain
Guard null ref and verify useGSAP dependency signature
Prevent null target errors and use the current @gsap/react signature that uses an options object for dependencies (if on v2+).
- useGSAP(() => {
- if (!fontLoaded) return;
-
- const timeline = gsap.timeline();
- timeline.fromTo(titleRef.current, { opacity: 0, y: 50 }, { opacity: 1, y: 0, duration: 2, ease: "expo.out" });
- }, [fontLoaded]);
+ useGSAP(() => {
+ if (!fontLoaded || !titleRef.current) return;
+ gsap.timeline().fromTo(
+ titleRef.current,
+ { opacity: 0, y: 50 },
+ { opacity: 1, y: 0, duration: 2, ease: "expo.out" }
+ );
+ }, { dependencies: [fontLoaded] });If you’re on an older @gsap/react, keep the array deps and only add the null-guard.
To confirm plugin version and whether useGSAP is registered globally:
🏁 Script executed:
#!/bin/bash
# Show @gsap/react version and check for registration
fd -a package.json | xargs -I {} jq -r '.dependencies["@gsap/react"] // .devDependencies["@gsap/react"] // empty + " (file: {})"' {}
rg -nP 'registerPlugin\(\s*useGSAP\s*\)' -C2 --type ts --type tsxLength of output: 241
Guard null refs and migrate to v2 useGSAP signature
useGSAP(() => {
- if (!fontLoaded) return;
-
- const timeline = gsap.timeline();
- timeline.fromTo(titleRef.current, { opacity: 0, y: 50 }, { opacity: 1, y: 0, duration: 2, ease: "expo.out" });
-}, [fontLoaded]);
+ if (!fontLoaded || !titleRef.current) return;
+ gsap.timeline().fromTo(
+ titleRef.current,
+ { opacity: 0, y: 50 },
+ { opacity: 1, y: 0, duration: 2, ease: "expo.out" }
+ );
+}, { dependencies: [fontLoaded] });Aligns with @gsap/react 2.1.2’s object‐based deps API and prevents null‐target errors.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useGSAP(() => { | |
| if (!fontLoaded) return; | |
| const timeline = gsap.timeline(); | |
| timeline.fromTo(titleRef.current, { opacity: 0, y: 50 }, { opacity: 1, y: 0, duration: 2, ease: "expo.out" }); | |
| }, [fontLoaded]); | |
| useGSAP(() => { | |
| if (!fontLoaded || !titleRef.current) return; | |
| gsap.timeline().fromTo( | |
| titleRef.current, | |
| { opacity: 0, y: 50 }, | |
| { opacity: 1, y: 0, duration: 2, ease: "expo.out" } | |
| ); | |
| }, { dependencies: [fontLoaded] }); |
🤖 Prompt for AI Agents
In mosu-app/src/widgets/home/YoutubeHeroSection.tsx around lines 18 to 23,
update the useGSAP invocation to the v2 object-based signature and guard against
null refs: change the hook call to useGSAP({ callback: () => { if (!fontLoaded
|| !titleRef.current) return; const timeline = gsap.timeline();
timeline.fromTo(titleRef.current, { opacity: 0, y: 50 }, { opacity: 1, y: 0,
duration: 2, ease: "expo.out" }); }, deps: [fontLoaded] }); ensuring you check
titleRef.current exists before calling fromTo so no null-target errors occur.
| <article className={styles.yt_hero_section__video}> | ||
| <YouTubeEmbed videoid={"sy0h73_DO5M"} style="margin: 0px auto;" /> | ||
| </article> |
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.
Fix invalid React style prop (string passed instead of object)
Passing a string to style will error; use a style object.
- <article className={styles.yt_hero_section__video}>
- <YouTubeEmbed videoid={"sy0h73_DO5M"} style="margin: 0px auto;" />
- </article>
+ <article className={styles.yt_hero_section__video}>
+ <YouTubeEmbed
+ videoid="sy0h73_DO5M"
+ style={{ margin: "0 auto" }}
+ />
+ </article>Optional hardening (keeps viewers on your content):
+ <YouTubeEmbed
+ videoid="sy0h73_DO5M"
+ style={{ margin: "0 auto" }}
+ params="rel=0&modestbranding=1&playsinline=1"
+ />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <article className={styles.yt_hero_section__video}> | |
| <YouTubeEmbed videoid={"sy0h73_DO5M"} style="margin: 0px auto;" /> | |
| </article> | |
| <article className={styles.yt_hero_section__video}> | |
| <YouTubeEmbed | |
| videoid="sy0h73_DO5M" | |
| style={{ margin: "0 auto" }} | |
| /> | |
| </article> |
🤖 Prompt for AI Agents
In mosu-app/src/widgets/home/YoutubeHeroSection.tsx around lines 32 to 34, the
YouTubeEmbed component is being passed a string for the style prop which will
error in React; change the prop to a proper style object (e.g., an object with
margin set to "0px auto") or remove the prop if unnecessary, and ensure the prop
type matches React.CSSProperties if the component is typed.
| <Link href="/apply" scroll> | ||
| <button ref={registerButtonRef}> | ||
| <span>지금 바로 신청하기</span> | ||
| <span> | ||
| <ChevronRight strokeWidth={1} /> | ||
| </span> | ||
| </button> | ||
| </Link> |
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.
Avoid interactive-in-interactive: don’t nest a button inside Link
Anchor-with-button is invalid and harms a11y. Make the Link’s anchor the CTA.
- <Link href="/apply" scroll>
- <button ref={registerButtonRef}>
- <span>지금 바로 신청하기</span>
- <span>
- <ChevronRight strokeWidth={1} />
- </span>
- </button>
- </Link>
+ <Link href="/apply" scroll>
+ <span>지금 바로 신청하기</span>
+ <span>
+ <ChevronRight strokeWidth={1} />
+ </span>
+ </Link>Pair with the SCSS change that styles a { ... } instead of button { ... }.
🤖 Prompt for AI Agents
In mosu-app/src/widgets/home/YoutubeHeroSection.tsx around lines 41–48, a
<button> is nested inside a Next.js <Link>, which creates an interactive element
inside another interactive element; replace the inner <button> with an anchor
element used as the CTA (so the Link's anchor is the interactive element), move
the existing ref (registerButtonRef) from the button to the anchor, preserve the
inner spans/content as-is, ensure the anchor gets the same className/CSS hook
and any ARIA attributes needed (or add an aria-label) so accessibility/keyboard
focus is preserved, and update the SCSS to style the anchor selector instead of
button.
| <footer className={styles.yt_hero_section__footer}> | ||
| <ChevronDown size={30} /> | ||
| </footer> |
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.
CSS module class not defined
styles.yt_hero_section__footer isn’t declared in the SCSS. Either add that class in the stylesheet or remove the className here.
- <footer className={styles.yt_hero_section__footer}>
+ <footer>
<ChevronDown size={30} />
</footer>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <footer className={styles.yt_hero_section__footer}> | |
| <ChevronDown size={30} /> | |
| </footer> | |
| <footer> | |
| <ChevronDown size={30} /> | |
| </footer> |
🤖 Prompt for AI Agents
In mosu-app/src/widgets/home/YoutubeHeroSection.tsx around lines 50 to 52, the
JSX uses styles.yt_hero_section__footer but that class is not defined in the
SCSS module; fix by either adding the yt_hero_section__footer definition to the
corresponding .module.scss with the desired styles (matching naming convention
and export) or remove the className prop from the footer element so it relies on
default styles—ensure imports remain consistent and run the build to verify no
CSS-module type errors.
✅ Linked Issue
Summary by CodeRabbit