feat(frontend): add reusable executive info cards#68
Conversation
ashoomky
left a comment
ashoomky
left a comment
There was a problem hiding this comment.
Great work Hayden! Just some minor things to fix up:
Two font fixes needed: both the role label and name <h3> use font-heading (Staatliches) but Figma specifies Inter, so make sure to swap both to font-body with the correct weight. While you're on the role label, change break-all to break-words to fix the mid-word splitting.
Also either add a bio prop and render it, or remove the field from the schema, as right now it collects data that goes nowhere. The string branch in the photo type can also be dropped since depth is always ≥ 1 on fetch.
For the executives/page.tsx, delete the ExecutiveCardData type and import Executive from @/payload-types instead, as it's already generated from the schema and won't drift when fields change.
Lastly, for proper conventions make sure to create a ExecCard folder, and put the ExecCard.tsx file in that folder. Refer to Navbar for example.
| type ExecutiveCardData = { | ||
| id: string | ||
| role: string | ||
| name: string | ||
| degree?: string | null | ||
| position?: string | null | ||
| yearsOfExperience?: number | null | ||
| photo?: | ||
| | { | ||
| url?: string | null | ||
| alt?: string | null | ||
| } | ||
| | string | ||
| | null | ||
| } |
There was a problem hiding this comment.
This type already exists as Executive in payload-types.ts. Payload generates it automatically from the collection schema. If a field is added or changed in the schema, payload-types.ts updates but this type will silently drift. Delete this block and import Executive from @/payload-types instead.
| limit: 100, | ||
| }) | ||
|
|
||
| const executives = result.docs as ExecutiveCardData[] |
There was a problem hiding this comment.
Change to result.docs as Executive[] once the manual type above is removed
| photo?: | ||
| | { | ||
| url?: string | null | ||
| alt?: string | null | ||
| } | ||
| | string | ||
| | null | ||
| } |
There was a problem hiding this comment.
The string branch here will never be hit in practice. Payload always returns a populated media object or null for an upload field, never a raw URL string. The union adds complexity without benefit, so to fix this simplify to { url?: string | null; alt?: string | null } | null.
|
|
||
| return ( | ||
| <article className="mx-auto flex w-full max-w-[220px] flex-col items-center text-center"> | ||
| <p className="mb-3 max-w-full break-all text-center font-heading text-[#7FA6D8] text-sm uppercase tracking-wide sm:text-base"> |
There was a problem hiding this comment.
Two issues here: break-all splits at any character boundary which causes the ugly mid-word wrapping seen in testing (where you had BBBBBBB). Change to break-words.
Also the font for people's names and their roles, should be font Inter as per Figma, right now it's Staatliches. Fix this please!
| )} | ||
| </div> | ||
|
|
||
| <h3 className="mb-2 font-heading text-brand-primary text-lg uppercase leading-tight sm:text-xl"> |
There was a problem hiding this comment.
Same font issue as the role label, font-heading resolves to Staatliches but Figma uses Inter here. Change to font-body font-bold (or whichever weight the spec calls for).
| {degree ? ( | ||
| <p className="font-body text-brand-primary text-sm leading-snug">{degree}</p> | ||
| ) : null} | ||
| {position ? ( | ||
| <p className="font-body text-brand-navy text-sm leading-snug">{position}</p> | ||
| ) : null} | ||
| {typeof yearsOfExperience === "number" ? ( | ||
| <p className="font-body text-brand-navy text-sm leading-snug"> | ||
| {yearsOfExperience} years exp. | ||
| </p> | ||
| ) : null} |
There was a problem hiding this comment.
The Executives collection defines a bio field but it's not in ExecCardProps and isn't rendered here, so anything entered in the CMS for bio is silently discarded. Either add a bio?: string | null prop and render it, or remove the field from the collection config.
2 participants
Description
Implements a reusable executive member info card for the About page.
This PR:
ExecCardcomponentdegree,position,yearsOfExperience, andphoto)/executivespage for testingCloses #59
Type of change
How Has This Been Tested?
Tested:
Checklist before requesting a review