Teams Page Redesign#58
Conversation
✅ Deploy Preview for soscv2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
VivekNeer
left a comment
There was a problem hiding this comment.
PR #58 — Full Code Review Notes
Branch:
PranamNK:teams→so-sc:main
Reviewer: VivekNeer
Files reviewed: All 14 code/config changed files (excluding images)
Issues Found
Critical — Build/Runtime
1. Wrong GSAP import path (3 files)
All three files import from gsap/dist/ScrollTrigger — the UMD bundle which uses self (browser-only global) and crashes Node.js in dev mode.
Files:
src/components/ui/fade-in.tsxsrc/components/ui/text-block-animation.tsxsrc/components/ui/executive-impact-carousel.tsx
- import { ScrollTrigger } from "gsap/dist/ScrollTrigger";
+ import { ScrollTrigger } from "gsap/ScrollTrigger";See pr-58-gsap-bug.md for full analysis.
2. noExternal workaround in astro.config.mjs should be removed [continuation of the first issue]
They added this to paper over issue #1:
vite: {
ssr: {
noExternal: ["gsap", "@gsap/react"],
},
},Once the import paths are fixed, this should be removed. Leaving it in adds unnecessary bundling overhead and masks the wrong import paths.
Design — Violates JustModo's Requested Changes
3. Rounded corners still present everywhere
rounded and rounded-full Tailwind classes are still used across components despite being explicitly requested to be removed.
Locations:
TeamsCard.astroline:class="... rounded bg-[#F0FFF4] ..."TeamView.tsxMemberCard:className="... rounded bg-[#F0FFF4] ..."TeamView.tsxtab buttons:className="... rounded-full border-2 ..."TeamView.tsxsocial icon links:className="... rounded bg-[#3ce56e] ..."TeamsCard.astrosocial icon links:class="... rounded bg-[#3ce56e] ..."
4. Hover shadow still present on cards
TeamView.tsx MemberCard still has:
hover:shadow-lg
JustModo specifically asked for this to be removed.
5. Heavy green background on mobile landing
TeamLanding.astro carousel wrapper uses bg-[#0ade4a] (solid bright green) as the background on mobile:
<div class="... bg-[#0ade4a] ...">JustModo asked for softer colors — more white. The desktop side (md:bg-[#E6FDE2]) is a soft green which is better, but mobile is still pure green behind the overlay.
6. Flash of invisible content on initial page load
On first load, the entire right column (the "MEET THE MINDS BEHIND THE CODE" text) is blank for a noticeable moment before appearing. This is caused by opacity: 0 being baked into the server-rendered HTML as an inline style, so the browser renders the page with the text hidden before JS even loads.
In fade-in.tsx — the wrapper div has style={{ opacity: 0 }} in JSX:
// ❌ This gets SSR'd into the HTML — text is invisible before JS loads
<div ref={containerRef} style={{ opacity: 0 }}>
{children}
</div>Additionally, gsap.fromTo forces the element back to opacity: 0 the moment GSAP runs, causing a second reverse flash even after the fix above.
In text-block-animation.tsx — same problem on contentRef's div:
// ❌ Same issue
<div ref={contentRef} style={{ opacity: 0 }}>
{children}
</div>GSAP already sets opacity: 0 inside useGSAP via gsap.set(contentRef.current, { opacity: 0 }) before the animation runs — so the inline style in JSX is redundant and harmful.
Fix for fade-in.tsx:
- Remove
style={{ opacity: 0 }}from the JSX div - Change
gsap.fromTotogsap.fromso GSAP animates fromopacity: 0to the element's natural state, instead of forcing it invisible first:
- gsap.fromTo(
- containerRef.current,
- { opacity: 0, y: yOffset },
- {
- opacity: 1,
- y: 0,
- duration: duration,
- ease: "power2.out",
- delay: delay,
- scrollTrigger: { ... },
- },
- );
+ gsap.from(containerRef.current, {
+ opacity: 0,
+ y: yOffset,
+ duration: duration,
+ ease: "power2.out",
+ delay: delay,
+ scrollTrigger: { ... },
+ });
- <div ref={containerRef} style={{ opacity: 0 }}>
+ <div ref={containerRef}>Fix for text-block-animation.tsx:
- <div ref={contentRef} style={{ opacity: 0 }}>
+ <div ref={contentRef}>The gsap.set(contentRef.current, { opacity: 0 }) call inside useGSAP already handles the initial hidden state on the client side — the inline style is not needed.
⚠️ Watch out: When removing thestyle={{ opacity: 0 }}from thecontentRefdiv intext-block-animation.tsx, make sure therefstays asref={contentRef}and does not accidentally becomeref={containerRef}. If both the outer and inner divs point tocontainerRef, thencontentRef.currentwill benull, the GSAP guardif (!contentRef.current) returnwill bail, and the entire animation will silently stop working.
7. Spacing at bottom of parallax
TeamLanding.astro outer section has pb-20 on mobile inside the text column:
class="... pb-20 sm:px-10 ..."This is likely contributing to the extra space JustModo flagged at the bottom.
Code Quality
8. isMobile state in carousel is computed but never used
In executive-impact-carousel.tsx:
const [isMobile, setIsMobile] = React.useState(false);
React.useEffect(() => {
const checkMobile = () => setIsMobile(window.innerWidth < 1024);
...
}, []);The comment even says "Mobile/Tablet: 2 Columns" but the actual layout code always does 3 columns regardless of isMobile:
// Always 3 columns — isMobile is never used to change this
col1Members = safeMembers.filter((_, i) => i % 3 === 0);
col2Members = safeMembers.filter((_, i) => i % 3 === 1);
col3Members = safeMembers.filter((_, i) => i % 3 === 2);And the JSX hardcodes gridTemplateColumns: "repeat(3, 1fr)". Dead state — either implement the 2-column mobile layout or remove the isMobile logic.
9. Array index used as React key
In TeamView.tsx and executive-impact-carousel.tsx, array indices are used as keys:
{executiveMembers.map((member, idx) => (
<MemberCard key={idx} member={member} />
))}Should use a stable unique identifier like member.username or member.name instead.
10. Duplicate TeamMember type definition
TeamMember is defined in two places:
src/data/team/type.ts— the Zod schema + inferred type (the correct source of truth)src/components/team/TeamView.tsx— a separate local interface that duplicates the shape
TeamView.tsx should import from type.ts instead of defining its own.
11. team.astro page deleted but route not redirected
The old src/pages/team.astro (at route /team) has been deleted and replaced with src/pages/team/index.astro. This is the same route so it works fine — but the old ProfileCard.astro component referenced in the deleted file may now be unused. Worth checking if ProfileCard.astro can be cleaned up.
12. biome.json — tab/space formatting change
The entire biome.json was reformatted from tabs to spaces (or vice versa), producing a noisy 182-line diff with zero functional changes. This makes reviewing the actual config changes harder than it needs to be.
What's Good
- The year-based tab filtering in
TeamView.tsxis a solid improvement over the old flat list - Zod schema in
type.tsis a good addition — proper runtime validation on team data - The
mix-blend-multiplyapproach for transparent PNG team photos on white background is correct - GSAP animation architecture (ScrollTrigger + timeline) is clean conceptually, just needs the right import path
data/team/split into separate JSON files per batch is well organized- New
batch2027.jsonand updatedalumni.json/coordinators.jsonkeep data current
Commit Message Compliance
CONTRIBUTING.md specifies a strict uppercase prefix format:
PREFIX: short description
Valid prefixes: ADD, FEAT, FIX, SFIX, TYPO, RFT, WIP, INIT
Of the 23 commits in this PR, only 4 follow the format correctly. Here's the full breakdown:
| Commit | Message | Issue |
|---|---|---|
c06e5fd |
Refactor team page into separate folder |
No prefix — should be RFT: |
9b53bea |
teams added |
No prefix, vague |
113e59a |
created new files |
No prefix, vague |
505a4ef |
ADD: team member photos without background |
✅ Correct |
3a7a98a |
ADD: 2 member photos |
✅ Correct |
16a81ae |
added TeamCard, Updated batch2027, added linkdin ids |
No prefix — should be ADD: or FEAT: |
e0739f6 |
ADD: member closeup portraits |
✅ Correct |
bcf2438 |
fix: card image layout and alumini part layout |
Lowercase fix — should be FIX: |
363c769 |
landing page |
No prefix, not descriptive |
e7af8c4 |
Landing |
No prefix, single word — not acceptable |
48d0c23 |
FIX: fixed team-landing layout |
✅ Correct |
6efea30 |
landing animation completed |
No prefix — should be FEAT: |
ade3804 |
add years field with appropriate values |
No prefix — should be ADD: |
75a8f3e |
work review, modifications and sync |
No prefix, vague |
f799eaf |
rearranged the names and years in json |
No prefix — should be RFT: |
763ad17 |
teams page completed |
No prefix — should be FEAT: |
e32bf8b |
teams page few image changes |
No prefix — should be ADD: |
9394afa |
prettier to fix formatting |
No prefix — should be FIX: |
044553f |
fix- update GSAP imports for Astro production build |
Lowercase + wrong delimiter (- instead of :) |
e0df7ff |
fix: update GSAP import in fade-in component for Astro build |
Lowercase fix — should be FIX: |
5fa2aa6 |
fix:gsp fixes |
Lowercase, missing space after :, "gsp" is a typo of "gsap" |
Summary: 4/21 non-merge commits follow the format. The three main issues:
- No prefix — most commits skip the prefix entirely
- Lowercase prefix —
fix:instead ofFIX: - Wrong delimiter or missing space —
fix-orfix:gspinstead ofFIX: gsap
Please squash or amend commit messages to follow the convention before merging, or at minimum ensure future commits on this branch follow it.
Summary for Review Comment
Request Changes on:
- Fix
gsap/dist/ScrollTrigger→gsap/ScrollTriggerin all 3 files + removenoExternalfrom astro.config - Remove
rounded/rounded-fullfrom cards and buttons - Remove
hover:shadow-lgfromMemberCard - Soften mobile background — replace
bg-[#0ade4a]withbg-whiteorbg-[#E6FDE2] - Fix flash of invisible content — remove
style={{ opacity: 0 }}from JSX infade-in.tsxandtext-block-animation.tsx, switchgsap.fromTotogsap.frominfade-in.tsx - Fix or remove the dead
isMobilestate in carousel - Fix commit messages — 17/21 commits violate the
PREFIX: descriptionformat fromCONTRIBUTING.md
Nice-to-have (can be a follow-up PR):
- Use stable keys instead of array indices
- Remove duplicate
TeamMemberinterface fromTeamView.tsx - Check if
ProfileCard.astrois now unused
|
@PranamNK are you still working on this PR? |
|
@JustModo i didn't start working on it need some time. |
502b922 to
2a0f7b6
Compare
| "designation": "Executive Member", | ||
| "email": "", | ||
| "linkedin": "", | ||
| "linkedin": "https://www.linkedin.com/in/hitha-badikillaya", |
There was a problem hiding this comment.
The url is invalid. Replace it with www.linkedin.com/in/hithabadikillaya
…to teams # Conflicts: # astro.config.mjs # biome.json
|
@HithaBadikillaya thanks for the review, i replaced the invalid url with the valid one, also aligned the community lead and the faculty co-ordinator cards sections properly so they sit on the same horizontal level. |



A visual and responsive upgrade to the Teams page to make it more dynamic and mobile-friendly.
Dynamic Header Animation: The "MEET THE MINDS BEHIND THE CODE" text now uses GSAP to reveal itself with animated green and black color blocks on load and scroll.
Modern Mobile Layout: Redesigned the mobile landing view into a single full screen section. The image carousel now scrolls infinitely in the background behind a dark overlay, with the header text sitting clearly on top.
Fixed Image Colors: Applying pure white backgrounds and correct blend modes behind the transparent PNGs.
Responsive Grid Adjustments: The Team View grid now automatically collapses into 2 perfectly sized columns on mobile, preventing team cards from becoming way too large.
Stable Production Builds: Fixed an SSR module export error in Astro's config to ensure GSAP animations build perfectly on GitHub Actions without failing.