Conversation
carlagn
requested review from
AmanVarshney01,
ArthurGamby,
aidankmcalister,
ankur-arch,
mhartington and
mhessdev
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Contributor
WalkthroughThis pull request introduces a comprehensive careers page featuring multiple new UI components for animations, carousels, and job listings. It adds five new npm dependencies for React animations and responsive layouts, extends the navigation system with variant styling support, and adds a reusable Quote component to the shared UI library. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (15)
apps/site/src/components/navigation-wrapper.tsx (1)
6-20: Avoid duplicating theLinkcontract locally.This local
Linkinterface can drift from the source type in@prisma-docs/ui/components/web-navigation. Prefer importing the shared type.Proposed refactor
import { WebNavigation } from "@prisma-docs/ui/components/web-navigation"; +import type { Link } from "@prisma-docs/ui/components/web-navigation"; import { usePathname } from "next/navigation"; -interface Link { - text: string; - external?: boolean; - url?: string; - icon?: string; - desc?: string; - col?: number; - sub?: Array<{ - text: string; - external?: boolean; - url: string; - icon?: string; - desc?: string; - }>; -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/navigation-wrapper.tsx` around lines 6 - 20, You defined a local Link interface in navigation-wrapper.tsx that duplicates the shared type; replace it by importing the canonical Link type from `@prisma-docs/ui/components/web-navigation` (or the exported name used there) and remove the local interface declaration so the component uses the imported Link type (update any usages in this file to reference the imported symbol).apps/site/src/components/careers/worldmap.module.css (1)
1-189: Consider using CSS custom properties for maintainability.The staggered animation pattern works well. For easier maintenance, you could extract the repeated colors into CSS custom properties at the
.worldmaplevel:.worldmap { --fill-primary: rgb(90, 103, 216); --fill-secondary: rgb(255, 255, 255); --fill-accent: rgb(163, 191, 250); }This is optional—the current implementation functions correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/worldmap.module.css` around lines 1 - 189, Extract repeated color values into CSS custom properties on the .worldmap selector (e.g., --fill-primary, --fill-secondary, --fill-accent) and replace the hardcoded rgb(...) values used in the .worldmap.active .svg-elem-2, .svg-elem-3, .svg-elem-4, .svg-elem-5, etc. selectors with those variables (var(--fill-...)); keep the transition and timing on .worldmap .svg-elem-N as-is so only the fill values are centralized for maintainability.apps/site/src/components/careers/masonry-client.tsx (1)
5-11: Consider adding a loading fallback to prevent layout shift.When the masonry component loads asynchronously, there's no placeholder content. This can cause layout shift (CLS), which affects Core Web Vitals. A skeleton or placeholder could improve perceived performance.
const MasonryPict = dynamic( () => import("./masonry").then((mod) => ({ default: mod.MasonryPict, })), - { ssr: false }, + { + ssr: false, + loading: () => <div className="min-h-[400px]" /> + }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/masonry-client.tsx` around lines 5 - 11, The dynamic import of MasonryPict currently has no loading fallback and can cause layout shift; update the dynamic call that defines MasonryPict to pass a loading option (e.g., loading: () => <Skeleton/> or a simple placeholder element) so a stable skeleton/placeholder is rendered while the async component (MasonryPict) loads, ensuring the placeholder mimics the component’s dimensions to avoid CLS.apps/site/src/components/careers/open-roles.tsx (2)
14-14: Heavy use ofanytypes reduces type safety.Multiple
anyannotations bypass TypeScript's benefits. Consider defining interfaces for the job data structure and component props for better maintainability and catching issues at compile time.interface Job { id: string; name: string; url: string; department: { label: string }; workLocation?: { label: string }; dept?: string; } interface OpenRolesMenuProps { setFilter: (value: string) => void; filters: string[]; }Also applies to: 45-52, 56-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/open-roles.tsx` at line 14, Replace the broad any annotations by defining concrete types and applying them to the component and related variables: create an OpenRolesMenuProps interface (with setFilter: (value: string) => void and filters: string[]) and a Job interface (id, name, url, department: { label: string }, optional workLocation, optional dept) and use these to type the OpenRolesMenu props and any job-related variables/arrays used inside map/rendering (e.g., job items, filters handling). Update other occurrences of any in the same file (around the areas mentioned, e.g., where jobs are mapped or filter state is used) to use these interfaces so setFilter, filters, and job properties are strongly typed.
86-86:Object.keys(jobs).lengthis falsy when zero—renders nothing without feedback.When there are no jobs, or while loading, the UI shows nothing. Consider adding a loading indicator and an empty state message.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/open-roles.tsx` at line 86, The conditional using Object.keys(jobs).length renders nothing when jobs is empty or loading; update the OpenRoles render logic to explicitly check for three states: loading, empty, and populated—replace the truthy check Object.keys(jobs).length with a clear condition like Object.keys(jobs).length > 0 for the populated branch, add a loading indicator (spinner or "Loading..." JSX) when an isLoading prop/state is true (or while jobs is undefined/null), and render an empty-state message/component when Object.keys(jobs).length === 0; reference the jobs variable and the OpenRoles component/render function when making these changes.apps/site/src/app/global.css (1)
196-198: Global Swiper override may affect other carousels.This rule applies to all
.swiper-button-prevelements site-wide. If another Swiper instance is added elsewhere, it will inherit this positioning. Consider scoping it to the testimonials carousel specifically:.testimonials-swiper .swiper-button-prev { top: 100% !important; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/global.css` around lines 196 - 198, The global CSS rule targets all .swiper-button-prev and will affect every Swiper instance; scope this override to only the testimonials carousel by replacing the selector with a more specific one such as .testimonials-swiper .swiper-button-prev (and mirror for .swiper-button-next if needed) so the positioning change only applies to the testimonials Swiper.apps/site/src/components/careers/testimonials.tsx (2)
60-64: Usingtestimonial.bodyas key could cause issues with duplicate content.If two testimonials ever have identical body text, React will have key collisions. Consider using a more stable identifier or combining with the index.
💡 Safer key approach
- {testimonials.map((testimonial: Testimonial) => ( - <SwiperSlide key={testimonial.body} className="my-auto"> + {testimonials.map((testimonial: Testimonial, index: number) => ( + <SwiperSlide key={`${index}-${testimonial.body.slice(0, 20)}`} className="my-auto">Or better yet, if you control the data shape, add an
idfield to each testimonial.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/testimonials.tsx` around lines 60 - 64, The current map uses the testimonial.body string as the React key which can collide when bodies are identical; update the mapping in testimonials.map to use a stable unique identifier (e.g., testimonial.id) for the SwiperSlide key, falling back to a combo like `${testimonial.id || testimonial.author}-${index}` or the index only if no id exists; modify the Testimonial data shape to include an id when possible and update the SwiperSlide key prop and any related usages of Testimonial to reference that id.
2-2: Unused import:useRefis imported butReact.useRefis used instead.Line 2 imports
useRefdirectly, but line 28 usesReact.useRef. Pick one approach for consistency and remove the unused import.🧹 Clean up imports
"use client"; -import { useRef } from "react"; import { Navigation, Pagination, Scrollbar, A11y } from "swiper/modules"; import { Swiper, SwiperSlide } from "swiper/react"; import type { Swiper as SwiperType } from "swiper";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/testimonials.tsx` at line 2, The import of useRef is unused because the code uses React.useRef; fix by making the usage and import consistent—either remove the named import useRef from the import list and keep React.useRef usage, or change React.useRef to useRef and keep the named import; update the import/export line accordingly and remove the unused identifier so there are no lint warnings related to useRef (reference symbols: useRef import and React.useRef usage in this component).apps/site/src/components/careers/flexible.module.css (1)
33-36: Floating-point precision artifacts in transition delays are cosmetic noise.Values like
0.07500000000000001sare functionally identical to0.075s, but they suggest this CSS was generated programmatically. If you're maintaining this manually going forward, consider cleaning up these values for readability. If it's generated, this is fine to leave as-is./* Current */ transition: fill 0.6s cubic-bezier(0.47, 0, 0.745, 0.715) 0.07500000000000001s; /* Cleaner */ transition: fill 0.6s cubic-bezier(0.47, 0, 0.745, 0.715) 0.075s;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/flexible.module.css` around lines 33 - 36, The transition declarations in flexible.module.css contain floating-point artifact delays (e.g. 0.07500000000000001s); update both the -webkit-transition and transition declarations to use the cleaned value 0.075s instead of the long floating-point representation so the CSS reads clearly and behaves identically (look for the transition and -webkit-transition properties in flexible.module.css and replace the long decimal delay).apps/site/src/components/careers/masonry.tsx (1)
2-3: Remove the@ts-ignorecomment since types are now declared.You've added type declarations in
apps/site/src/types/react-responsive-masonry.d.ts, so the ignore directive is no longer necessary and may mask future type issues.🧹 Suggested fix
"use client"; -// `@ts-ignore` - no types available for react-responsive-masonry import Masonry, { ResponsiveMasonry } from "react-responsive-masonry";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/masonry.tsx` around lines 2 - 3, Remove the obsolete "// `@ts-ignore` - no types available for react-responsive-masonry" comment above the import of Masonry and ResponsiveMasonry in the masonry.tsx file (where you import from "react-responsive-masonry"); since you added types in apps/site/src/types/react-responsive-masonry.d.ts, delete that ts-ignore and run TypeScript typecheck to confirm no errors, and also search for any other lingering ts-ignore comments related to react-responsive-masonry and remove them.apps/site/src/app/careers/page.tsx (4)
1-1: Unused import:Antigravityis imported but never used.The
Antigravitycomponent is imported from the homepage components but doesn't appear anywhere in the JSX.🧹 Remove unused import
-import Antigravity from "../../components/homepage/antigravity";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` at line 1, Remove the unused import statement for the Antigravity component: delete the line importing Antigravity (import Antigravity from "../../components/homepage/antigravity";) from page.tsx since the Antigravity symbol is not referenced in the JSX; alternatively, if the component is intended to be used, add it into the rendered JSX where appropriate (referencing the Antigravity component name) instead of leaving the unused import.
9-9: Unused import:reviewfrom homepage.json is not used.This import doesn't appear to be referenced anywhere in the component.
🧹 Remove unused import
-import review from "../../data/homepage.json";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` at line 9, The import "review" from "../../data/homepage.json" is unused in the careers page component; remove the unused import statement (the line importing review) from apps/site/src/app/careers/page.tsx, or if the data is actually needed, reference the symbol "review" in the component (e.g., within the CareersPage function) — but prefer deleting the import to eliminate the unused symbol.
192-201: Usenext/imageinstead of<img>for automatic optimization.Next.js's
Imagecomponent provides automatic lazy loading, responsive sizing, and format optimization. Using raw<img>tags bypasses these benefits.🖼️ Suggested refactor
+import Image from "next/image"; // In the JSX: - <img + <Image src="/illustrations/careers/hero_lines.svg" alt="Hero lines" + fill className="w-full h-full object-cover object-bottom hidden dark:block" /> - <img + <Image src="/illustrations/careers/hero_lines_light.svg" alt="Hero lines" + fill className="w-full h-full object-cover object-bottom block dark:hidden" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` around lines 192 - 201, Replace the raw <img> tags with Next.js' Image component: import Image from 'next/image' at the top of the file and swap each <img src=... alt=... className=...> for <Image> using the same src and alt, but use the fill prop (or explicit width/height) to preserve the original full-bleed behavior and keep the className (e.g., "object-cover object-bottom hidden dark:block" and "object-cover object-bottom block dark:hidden"); this enables automatic optimization, lazy-loading and responsive sizing for the two hero images.
324-327:dangerouslySetInnerHTMLis low-risk here but sets a risky pattern.The static analysis tool flagged this, and while the data is hardcoded in the same file (so XSS isn't an immediate concern), this pattern could be copied elsewhere with dynamic data. Consider using a React fragment with explicit elements instead:
💡 Safer alternative
Instead of HTML strings in the
benefitsarray:{ icon: "fa-regular fa-arrow-trend-up", title: "Stock options package", description: "with a maximum exercise period of 10 years after grant", }Then in the JSX:
<p className="text-lg"> <b>{benefit.title}</b> {benefit.description} </p>This eliminates the need for
dangerouslySetInnerHTMLentirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/careers/page.tsx` around lines 324 - 327, The code uses dangerouslySetInnerHTML on benefit.text which is risky and sets a pattern for future XSS; instead refactor the benefits data to store structured fields (e.g., title and description) and update the JSX that renders each benefit (the loop that references benefit and the <p> using dangerouslySetInnerHTML) to render elements directly (e.g., <b>{benefit.title}</b> and plain text for description) so you can remove dangerouslySetInnerHTML entirely and render with plain React nodes; update the benefits array entries and the renderer where benefit.text is used to reference benefit.title and benefit.description (or similar field names) and render them as normal JSX.apps/site/src/components/careers/stats-list.tsx (1)
6-11: Define a proper interface forstatsListitems instead of usingany.Using
any[]bypasses TypeScript's type checking entirely. This makes it easy to pass malformed data and harder to catch bugs at compile time. Consider defining an interface:🛠️ Suggested improvement
+"use client"; +import dynamic from "next/dynamic"; + +interface StatItem { + head: number; + sub: string; +} + const AnimatedNumbers = dynamic(() => import("react-animated-numbers"), { ssr: false, }); -export const StatsList = ({ statsList }: { statsList: any[] }) => { +export const StatsList = ({ statsList }: { statsList: StatItem[] }) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/careers/stats-list.tsx` around lines 6 - 11, The component currently accepts statsList: any[] which disables type checking; define a proper interface (e.g., interface StatItem { title: string; value: string | number; description?: string }) and change the StatsList prop type to statsList: StatItem[] and the map item type from any to StatItem, then update any property access in StatsList to use those typed fields (StatsList, statsList, StatItem are the identifiers to change) so TypeScript can validate the shape of each stat entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/site/src/app/careers/page.tsx`:
- Around line 182-185: The page's exported metadata uses homepage constants
(SITE_HOME_TITLE, SITE_HOME_DESCRIPTION) instead of careers-specific values;
update the metadata object exported as metadata to reference careers-specific
constants (e.g., CAREERS_TITLE and CAREERS_DESCRIPTION) or create and import
those new constants from the shared metadata/constants module, then replace
SITE_HOME_TITLE and SITE_HOME_DESCRIPTION with the new CAREERS_* constants in
the export.
In `@apps/site/src/components/careers/masonry.tsx`:
- Around line 20-28: The Image components inside the images.map in the masonry
component are using a hardcoded alt ("hello") which breaks accessibility; change
the alt prop to use the image metadata (e.g., replace alt="hello" with the
actual field from the mapped item such as e.imageAlt or e.altText) so each
<Image> renders a descriptive alt string from the images array used by the
masonry component.
In `@apps/site/src/components/careers/open-roles.tsx`:
- Line 67: The sort comparator currently uses a single-argument callback and
must be replaced with a two-argument comparator for stable ordering: update the
.sort call so the comparator accepts (a, b), computes whether a.name or b.name
includes "General Applications", returns positive if a is "General Applications"
and b is not (to push it after), negative if b is "General Applications" and a
is not (to keep normal items first), and returns 0 (or falls back to comparing
a.name and b.name) when both are the same type; modify the inline .sort((a: any)
=> ...) comparator accordingly.
- Around line 58-77: The fetchJobs function currently has no error handling—wrap
the async fetch and downstream processing in a try/catch inside fetchJobs, catch
network/JSON/parsing errors, and on error call a state updater (e.g.,
setJobs([]) and setJobsError or setJobsLoading(false)) so the component can
display an error or fallback UI; also log the caught error (console.error or
processLogger) and ensure you only call setJobs with the grouped result when the
fetch and mapping succeed (references: fetchJobs, setJobs, rdata,
rjobsWithDept).
In `@apps/site/src/components/careers/stats-list.tsx`:
- Line 22: The expression e.sub.toLowerCase() can throw if e.sub is
null/undefined; update the conditional in the stats-list rendering to guard or
normalize e.sub before calling toLowerCase (e.g., use optional chaining or
coerce to a string) so the check (previously written as e.sub.toLowerCase() ===
"remote") safely handles missing values; locate the conditional using the e.sub
symbol in the render JSX and replace it with a null-safe variant.
In `@apps/site/src/components/constants.ts`:
- Line 1: The BOARD_ID export can be undefined when NEXT_PUBLIC_BOARD_ID is not
set which leads to invalid fetch URLs (e.g., in open-roles.tsx); update the
export in constants.ts to validate at module load: check
process.env.NEXT_PUBLIC_BOARD_ID and either throw a clear error if missing or
provide a safe fallback value, then export the validated/normalized BOARD_ID so
consumers like open-roles.tsx always receive a defined string (reference symbol:
BOARD_ID).
In `@apps/site/src/components/navigation-wrapper.tsx`:
- Around line 37-41: The current exact path check in navigation-wrapper.tsx is
too strict and misses "/careers/", "/careers/anything" etc.; update the logic
that uses orm.includes(pathname.split("?")[0]) to normalize the path (strip
query and trailing slash) and perform prefix matching: e.g., compute const
pathnameNoQuery = pathname.split("?")[0].replace(/\/+$/,'') and then use
orm.some(p => pathnameNoQuery === p || pathnameNoQuery.startsWith(p + "/")) (or
specifically check pathnameNoQuery.startsWith("/careers")) so careers and nested
routes map to the "orm" variant instead of falling back to "ppg".
In `@apps/site/src/types/react-responsive-masonry.d.ts`:
- Around line 16-17: The declaration file references React.Component but never
imports React; add an explicit import of React (e.g. import * as React from
'react') at the top of the module declaration so React is in scope, then keep
the exported classes (ResponsiveMasonry and Masonry) as extending
React.Component; this ensures the symbols ResponsiveMasonry and Masonry compile
correctly in the declaration file.
In `@packages/ui/src/components/quote.tsx`:
- Around line 39-53: The Separator can render at the start when author.title is
falsy but author.company exists; update the JSX in the quote component so the
Separator is only rendered when both author.title and author.company are present
(i.e., wrap Separator in a conditional that checks both), leaving the existing
title and company spans unchanged; reference the author.title, author.company
checks and the Separator element in the component to locate and adjust the
rendering logic.
---
Nitpick comments:
In `@apps/site/src/app/careers/page.tsx`:
- Line 1: Remove the unused import statement for the Antigravity component:
delete the line importing Antigravity (import Antigravity from
"../../components/homepage/antigravity";) from page.tsx since the Antigravity
symbol is not referenced in the JSX; alternatively, if the component is intended
to be used, add it into the rendered JSX where appropriate (referencing the
Antigravity component name) instead of leaving the unused import.
- Line 9: The import "review" from "../../data/homepage.json" is unused in the
careers page component; remove the unused import statement (the line importing
review) from apps/site/src/app/careers/page.tsx, or if the data is actually
needed, reference the symbol "review" in the component (e.g., within the
CareersPage function) — but prefer deleting the import to eliminate the unused
symbol.
- Around line 192-201: Replace the raw <img> tags with Next.js' Image component:
import Image from 'next/image' at the top of the file and swap each <img src=...
alt=... className=...> for <Image> using the same src and alt, but use the fill
prop (or explicit width/height) to preserve the original full-bleed behavior and
keep the className (e.g., "object-cover object-bottom hidden dark:block" and
"object-cover object-bottom block dark:hidden"); this enables automatic
optimization, lazy-loading and responsive sizing for the two hero images.
- Around line 324-327: The code uses dangerouslySetInnerHTML on benefit.text
which is risky and sets a pattern for future XSS; instead refactor the benefits
data to store structured fields (e.g., title and description) and update the JSX
that renders each benefit (the loop that references benefit and the <p> using
dangerouslySetInnerHTML) to render elements directly (e.g.,
<b>{benefit.title}</b> and plain text for description) so you can remove
dangerouslySetInnerHTML entirely and render with plain React nodes; update the
benefits array entries and the renderer where benefit.text is used to reference
benefit.title and benefit.description (or similar field names) and render them
as normal JSX.
In `@apps/site/src/app/global.css`:
- Around line 196-198: The global CSS rule targets all .swiper-button-prev and
will affect every Swiper instance; scope this override to only the testimonials
carousel by replacing the selector with a more specific one such as
.testimonials-swiper .swiper-button-prev (and mirror for .swiper-button-next if
needed) so the positioning change only applies to the testimonials Swiper.
In `@apps/site/src/components/careers/flexible.module.css`:
- Around line 33-36: The transition declarations in flexible.module.css contain
floating-point artifact delays (e.g. 0.07500000000000001s); update both the
-webkit-transition and transition declarations to use the cleaned value 0.075s
instead of the long floating-point representation so the CSS reads clearly and
behaves identically (look for the transition and -webkit-transition properties
in flexible.module.css and replace the long decimal delay).
In `@apps/site/src/components/careers/masonry-client.tsx`:
- Around line 5-11: The dynamic import of MasonryPict currently has no loading
fallback and can cause layout shift; update the dynamic call that defines
MasonryPict to pass a loading option (e.g., loading: () => <Skeleton/> or a
simple placeholder element) so a stable skeleton/placeholder is rendered while
the async component (MasonryPict) loads, ensuring the placeholder mimics the
component’s dimensions to avoid CLS.
In `@apps/site/src/components/careers/masonry.tsx`:
- Around line 2-3: Remove the obsolete "// `@ts-ignore` - no types available for
react-responsive-masonry" comment above the import of Masonry and
ResponsiveMasonry in the masonry.tsx file (where you import from
"react-responsive-masonry"); since you added types in
apps/site/src/types/react-responsive-masonry.d.ts, delete that ts-ignore and run
TypeScript typecheck to confirm no errors, and also search for any other
lingering ts-ignore comments related to react-responsive-masonry and remove
them.
In `@apps/site/src/components/careers/open-roles.tsx`:
- Line 14: Replace the broad any annotations by defining concrete types and
applying them to the component and related variables: create an
OpenRolesMenuProps interface (with setFilter: (value: string) => void and
filters: string[]) and a Job interface (id, name, url, department: { label:
string }, optional workLocation, optional dept) and use these to type the
OpenRolesMenu props and any job-related variables/arrays used inside
map/rendering (e.g., job items, filters handling). Update other occurrences of
any in the same file (around the areas mentioned, e.g., where jobs are mapped or
filter state is used) to use these interfaces so setFilter, filters, and job
properties are strongly typed.
- Line 86: The conditional using Object.keys(jobs).length renders nothing when
jobs is empty or loading; update the OpenRoles render logic to explicitly check
for three states: loading, empty, and populated—replace the truthy check
Object.keys(jobs).length with a clear condition like Object.keys(jobs).length >
0 for the populated branch, add a loading indicator (spinner or "Loading..."
JSX) when an isLoading prop/state is true (or while jobs is undefined/null), and
render an empty-state message/component when Object.keys(jobs).length === 0;
reference the jobs variable and the OpenRoles component/render function when
making these changes.
In `@apps/site/src/components/careers/stats-list.tsx`:
- Around line 6-11: The component currently accepts statsList: any[] which
disables type checking; define a proper interface (e.g., interface StatItem {
title: string; value: string | number; description?: string }) and change the
StatsList prop type to statsList: StatItem[] and the map item type from any to
StatItem, then update any property access in StatsList to use those typed fields
(StatsList, statsList, StatItem are the identifiers to change) so TypeScript can
validate the shape of each stat entry.
In `@apps/site/src/components/careers/testimonials.tsx`:
- Around line 60-64: The current map uses the testimonial.body string as the
React key which can collide when bodies are identical; update the mapping in
testimonials.map to use a stable unique identifier (e.g., testimonial.id) for
the SwiperSlide key, falling back to a combo like `${testimonial.id ||
testimonial.author}-${index}` or the index only if no id exists; modify the
Testimonial data shape to include an id when possible and update the SwiperSlide
key prop and any related usages of Testimonial to reference that id.
- Line 2: The import of useRef is unused because the code uses React.useRef; fix
by making the usage and import consistent—either remove the named import useRef
from the import list and keep React.useRef usage, or change React.useRef to
useRef and keep the named import; update the import/export line accordingly and
remove the unused identifier so there are no lint warnings related to useRef
(reference symbols: useRef import and React.useRef usage in this component).
In `@apps/site/src/components/careers/worldmap.module.css`:
- Around line 1-189: Extract repeated color values into CSS custom properties on
the .worldmap selector (e.g., --fill-primary, --fill-secondary, --fill-accent)
and replace the hardcoded rgb(...) values used in the .worldmap.active
.svg-elem-2, .svg-elem-3, .svg-elem-4, .svg-elem-5, etc. selectors with those
variables (var(--fill-...)); keep the transition and timing on .worldmap
.svg-elem-N as-is so only the fill values are centralized for maintainability.
In `@apps/site/src/components/navigation-wrapper.tsx`:
- Around line 6-20: You defined a local Link interface in navigation-wrapper.tsx
that duplicates the shared type; replace it by importing the canonical Link type
from `@prisma-docs/ui/components/web-navigation` (or the exported name used there)
and remove the local interface declaration so the component uses the imported
Link type (update any usages in this file to reference the imported symbol).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 41db50da-e88a-40a5-ae56-7e2ad43901ea
⛔ Files ignored due to path filters (6)
apps/site/public/illustrations/careers/hero_lines.svgis excluded by!**/*.svgapps/site/public/illustrations/careers/hero_lines_light.svgis excluded by!**/*.svgapps/site/public/illustrations/careers/hero_lines_mobile.svgis excluded by!**/*.svgapps/site/public/illustrations/careers/hero_lines_mobile_light.svgis excluded by!**/*.svgapps/site/public/illustrations/careers/world_map_dots.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
apps/site/package.jsonapps/site/public/photos/careers/masonry_1.webpapps/site/public/photos/careers/masonry_10.webpapps/site/public/photos/careers/masonry_11.webpapps/site/public/photos/careers/masonry_12.webpapps/site/public/photos/careers/masonry_13.webpapps/site/public/photos/careers/masonry_14.webpapps/site/public/photos/careers/masonry_2.webpapps/site/public/photos/careers/masonry_3.webpapps/site/public/photos/careers/masonry_4.webpapps/site/public/photos/careers/masonry_5.webpapps/site/public/photos/careers/masonry_6.webpapps/site/public/photos/careers/masonry_7.webpapps/site/public/photos/careers/masonry_8.webpapps/site/public/photos/careers/masonry_9.webpapps/site/src/app/careers/page.tsxapps/site/src/app/global.cssapps/site/src/app/layout.tsxapps/site/src/components/careers/Challenges.tsxapps/site/src/components/careers/Flexible.tsxapps/site/src/components/careers/WorldMap.tsxapps/site/src/components/careers/challenges.module.cssapps/site/src/components/careers/flexible.module.cssapps/site/src/components/careers/masonry-client.tsxapps/site/src/components/careers/masonry.tsxapps/site/src/components/careers/open-roles.tsxapps/site/src/components/careers/stats-list.tsxapps/site/src/components/careers/testimonials.tsxapps/site/src/components/careers/worldmap.module.cssapps/site/src/components/constants.tsapps/site/src/components/homepage/card-section/card-section.tsxapps/site/src/components/navigation-wrapper.tsxapps/site/src/types/react-responsive-masonry.d.tspackages/ui/src/components/navigation-menu.tsxpackages/ui/src/components/quote.tsxpackages/ui/src/components/web-navigation.tsx
Comment on lines
+182
to
+185
| export const metadata: Metadata = { | ||
| title: SITE_HOME_TITLE, | ||
| description: SITE_HOME_DESCRIPTION, | ||
| }; |
Contributor
There was a problem hiding this comment.
Metadata uses homepage constants instead of careers-specific values.
The page uses SITE_HOME_TITLE and SITE_HOME_DESCRIPTION which are likely meant for the homepage. A careers page should have its own title and description for proper SEO.
📝 Suggested fix
export const metadata: Metadata = {
- title: SITE_HOME_TITLE,
- description: SITE_HOME_DESCRIPTION,
+ title: "Careers at Prisma | Join Our Team",
+ description: "Join Prisma and help us empower developers to build data-driven applications. View our open positions and benefits.",
};Or create dedicated constants like CAREERS_TITLE and CAREERS_DESCRIPTION in your metadata file.
📝 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.
Suggested change
| export const metadata: Metadata = { | |
| title: SITE_HOME_TITLE, | |
| description: SITE_HOME_DESCRIPTION, | |
| }; | |
| export const metadata: Metadata = { | |
| title: "Careers at Prisma | Join Our Team", | |
| description: "Join Prisma and help us empower developers to build data-driven applications. View our open positions and benefits.", | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/app/careers/page.tsx` around lines 182 - 185, The page's
exported metadata uses homepage constants (SITE_HOME_TITLE,
SITE_HOME_DESCRIPTION) instead of careers-specific values; update the metadata
object exported as metadata to reference careers-specific constants (e.g.,
CAREERS_TITLE and CAREERS_DESCRIPTION) or create and import those new constants
from the shared metadata/constants module, then replace SITE_HOME_TITLE and
SITE_HOME_DESCRIPTION with the new CAREERS_* constants in the export.
Comment on lines
+20
to
+28
| {images.map((e: any, idx: number) => ( | ||
| <Image | ||
| key={idx} | ||
| src={e.imageUrl} | ||
| width={400} | ||
| height={400} | ||
| alt="hello" | ||
| className="w-full rounded-lg shadow-[0px_18px_42px_0px_rgba(23,43,77,0.08),0px_4px_26px_0px_rgba(23,43,77,0.05),0px_0px_46px_0px_rgba(23,43,77,0.01)]" | ||
| /> |
Contributor
There was a problem hiding this comment.
Accessibility issue: Images have non-descriptive alt text.
The masonryImages data in page.tsx includes imageAlt values like "Masonry 1", "Masonry 14", but this component ignores them and uses alt="hello" for all images. Screen reader users will hear "hello" repeated for every image, which provides no useful context.
♿ Fix to use actual alt text
+interface MasonryImage {
+ imageUrl: string;
+ imageAlt: string;
+}
+
export const MasonryPict = ({
images,
gutter,
}: {
- images: any[];
+ images: MasonryImage[];
gutter: string;
}) => {
return (
<div className="w-full">
<div className="max-w-[1232px] w-full p-4 mx-auto [&_>_div_>_div]:items-center!">
<ResponsiveMasonry
columnsCountBreakPoints={{ 350: 2, 750: 3, 940: 4, 1124: 5 }}
>
<Masonry gutter={gutter} center>
- {images.map((e: any, idx: number) => (
+ {images.map((e, idx) => (
<Image
key={idx}
src={e.imageUrl}
width={400}
height={400}
- alt="hello"
+ alt={e.imageAlt}
className="w-full rounded-lg shadow-[0px_18px_42px_0px_rgba(23,43,77,0.08),0px_4px_26px_0px_rgba(23,43,77,0.05),0px_0px_46px_0px_rgba(23,43,77,0.01)]"
/>
))}
</Masonry>
</ResponsiveMasonry>
</div>
</div>
);
};📝 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.
Suggested change
| {images.map((e: any, idx: number) => ( | |
| <Image | |
| key={idx} | |
| src={e.imageUrl} | |
| width={400} | |
| height={400} | |
| alt="hello" | |
| className="w-full rounded-lg shadow-[0px_18px_42px_0px_rgba(23,43,77,0.08),0px_4px_26px_0px_rgba(23,43,77,0.05),0px_0px_46px_0px_rgba(23,43,77,0.01)]" | |
| /> | |
| interface MasonryImage { | |
| imageUrl: string; | |
| imageAlt: string; | |
| } | |
| export const MasonryPict = ({ | |
| images, | |
| gutter, | |
| }: { | |
| images: MasonryImage[]; | |
| gutter: string; | |
| }) => { | |
| return ( | |
| <div className="w-full"> | |
| <div className="max-w-[1232px] w-full p-4 mx-auto [&_>_div_>_div]:items-center!"> | |
| <ResponsiveMasonry | |
| columnsCountBreakPoints={{ 350: 2, 750: 3, 940: 4, 1124: 5 }} | |
| > | |
| <Masonry gutter={gutter} center> | |
| {images.map((e, idx) => ( | |
| <Image | |
| key={idx} | |
| src={e.imageUrl} | |
| width={400} | |
| height={400} | |
| alt={e.imageAlt} | |
| className="w-full rounded-lg shadow-[0px_18px_42px_0px_rgba(23,43,77,0.08),0px_4px_26px_0px_rgba(23,43,77,0.05),0px_0px_46px_0px_rgba(23,43,77,0.01)]" | |
| /> | |
| ))} | |
| </Masonry> | |
| </ResponsiveMasonry> | |
| </div> | |
| </div> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/components/careers/masonry.tsx` around lines 20 - 28, The Image
components inside the images.map in the masonry component are using a hardcoded
alt ("hello") which breaks accessibility; change the alt prop to use the image
metadata (e.g., replace alt="hello" with the actual field from the mapped item
such as e.imageAlt or e.altText) so each <Image> renders a descriptive alt
string from the images array used by the masonry component.
Comment on lines
+58
to
+77
| async function fetchJobs() { | ||
| const rdata = await fetch( | ||
| `https://api.rippling.com/platform/api/ats/v1/board/${BOARD_ID}/jobs`, | ||
| ).then((res) => res.json()); | ||
| const rjobsWithDept = rdata | ||
| .map((job: any) => { | ||
| job.dept = job.department.label; | ||
| return job; | ||
| }) | ||
| .sort((a: any) => (a.name.includes("General Applications") ? 1 : -1)); | ||
|
|
||
| const sameUrls = groupBy(rjobsWithDept, "url"); | ||
|
|
||
| const sameUrl = Object.keys(sameUrls); | ||
| const rjobsWithDeptSet = sameUrl.map((e: any) => sameUrls[e][0]); | ||
|
|
||
| const rjobsData = groupBy(rjobsWithDeptSet, "dept"); | ||
|
|
||
| setJobs(rjobsData); | ||
| } |
Contributor
There was a problem hiding this comment.
Missing error handling for fetch—failures silently leave jobs empty.
If the Rippling API is unavailable or returns an error, the component will silently fail with no feedback to users. Consider adding try/catch with error state.
🛡️ Proposed improvement
+ const [error, setError] = React.useState<string | null>(null);
+ const [loading, setLoading] = React.useState(true);
async function fetchJobs() {
+ try {
+ setLoading(true);
const rdata = await fetch(
`https://api.rippling.com/platform/api/ats/v1/board/${BOARD_ID}/jobs`,
).then((res) => res.json());
// ... rest of processing
setJobs(rjobsData);
+ } catch (err) {
+ setError("Failed to load job listings");
+ console.error(err);
+ } finally {
+ setLoading(false);
+ }
}📝 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.
Suggested change
| async function fetchJobs() { | |
| const rdata = await fetch( | |
| `https://api.rippling.com/platform/api/ats/v1/board/${BOARD_ID}/jobs`, | |
| ).then((res) => res.json()); | |
| const rjobsWithDept = rdata | |
| .map((job: any) => { | |
| job.dept = job.department.label; | |
| return job; | |
| }) | |
| .sort((a: any) => (a.name.includes("General Applications") ? 1 : -1)); | |
| const sameUrls = groupBy(rjobsWithDept, "url"); | |
| const sameUrl = Object.keys(sameUrls); | |
| const rjobsWithDeptSet = sameUrl.map((e: any) => sameUrls[e][0]); | |
| const rjobsData = groupBy(rjobsWithDeptSet, "dept"); | |
| setJobs(rjobsData); | |
| } | |
| const [error, setError] = React.useState<string | null>(null); | |
| const [loading, setLoading] = React.useState(true); | |
| async function fetchJobs() { | |
| try { | |
| setLoading(true); | |
| const rdata = await fetch( | |
| `https://api.rippling.com/platform/api/ats/v1/board/${BOARD_ID}/jobs`, | |
| ).then((res) => res.json()); | |
| const rjobsWithDept = rdata | |
| .map((job: any) => { | |
| job.dept = job.department.label; | |
| return job; | |
| }) | |
| .sort((a: any) => (a.name.includes("General Applications") ? 1 : -1)); | |
| const sameUrls = groupBy(rjobsWithDept, "url"); | |
| const sameUrl = Object.keys(sameUrls); | |
| const rjobsWithDeptSet = sameUrl.map((e: any) => sameUrls[e][0]); | |
| const rjobsData = groupBy(rjobsWithDeptSet, "dept"); | |
| setJobs(rjobsData); | |
| } catch (err) { | |
| setError("Failed to load job listings"); | |
| console.error(err); | |
| } finally { | |
| setLoading(false); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/components/careers/open-roles.tsx` around lines 58 - 77, The
fetchJobs function currently has no error handling—wrap the async fetch and
downstream processing in a try/catch inside fetchJobs, catch
network/JSON/parsing errors, and on error call a state updater (e.g.,
setJobs([]) and setJobsError or setJobsLoading(false)) so the component can
display an error or fallback UI; also log the caught error (console.error or
processLogger) and ensure you only call setJobs with the grouped result when the
fetch and mapping succeed (references: fetchJobs, setJobs, rdata,
rjobsWithDept).
| job.dept = job.department.label; | ||
| return job; | ||
| }) | ||
| .sort((a: any) => (a.name.includes("General Applications") ? 1 : -1)); |
Contributor
There was a problem hiding this comment.
Incorrect sort comparator—only receives one argument.
The sort callback (a) => (a.name.includes("General Applications") ? 1 : -1) ignores the second element b. A valid comparator must compare both elements and return negative, zero, or positive. This produces unpredictable ordering.
🐛 Proposed fix
- .sort((a: any) => (a.name.includes("General Applications") ? 1 : -1));
+ .sort((a: any, b: any) => {
+ const aIsGeneral = a.name.includes("General Applications");
+ const bIsGeneral = b.name.includes("General Applications");
+ if (aIsGeneral && !bIsGeneral) return 1;
+ if (!aIsGeneral && bIsGeneral) return -1;
+ return 0;
+ });📝 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.
Suggested change
| .sort((a: any) => (a.name.includes("General Applications") ? 1 : -1)); | |
| .sort((a: any, b: any) => { | |
| const aIsGeneral = a.name.includes("General Applications"); | |
| const bIsGeneral = b.name.includes("General Applications"); | |
| if (aIsGeneral && !bIsGeneral) return 1; | |
| if (!aIsGeneral && bIsGeneral) return -1; | |
| return 0; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/components/careers/open-roles.tsx` at line 67, The sort
comparator currently uses a single-argument callback and must be replaced with a
two-argument comparator for stable ordering: update the .sort call so the
comparator accepts (a, b), computes whether a.name or b.name includes "General
Applications", returns positive if a is "General Applications" and b is not (to
push it after), negative if b is "General Applications" and a is not (to keep
normal items first), and returns 0 (or falls back to comparing a.name and
b.name) when both are the same type; modify the inline .sort((a: any) => ...)
comparator accordingly.
| color: "var(--color-foreground-neutral)", | ||
| }} | ||
| /> | ||
| {e.sub.toLowerCase() === "remote" && `%`} |
Contributor
There was a problem hiding this comment.
Potential runtime error if e.sub is undefined or null.
Calling .toLowerCase() on an undefined/null value will throw a TypeError. While your current data appears safe, defensive coding would prevent future issues if the data shape changes.
🛡️ Safer approach
- {e.sub.toLowerCase() === "remote" && `%`}
+ {e.sub?.toLowerCase() === "remote" && `%`}📝 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.
Suggested change
| {e.sub.toLowerCase() === "remote" && `%`} | |
| {e.sub?.toLowerCase() === "remote" && `%`} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/components/careers/stats-list.tsx` at line 22, The expression
e.sub.toLowerCase() can throw if e.sub is null/undefined; update the conditional
in the stats-list rendering to guard or normalize e.sub before calling
toLowerCase (e.g., use optional chaining or coerce to a string) so the check
(previously written as e.sub.toLowerCase() === "remote") safely handles missing
values; locate the conditional using the e.sub symbol in the render JSX and
replace it with a null-safe variant.
| @@ -0,0 +1 @@ | |||
| export const BOARD_ID = process.env.NEXT_PUBLIC_BOARD_ID; | |||
Contributor
There was a problem hiding this comment.
Missing validation for undefined BOARD_ID could cause API failures.
When NEXT_PUBLIC_BOARD_ID is not set, this will be undefined, and the fetch URL in open-roles.tsx will contain the literal string "undefined". Consider adding a fallback or throwing a clear error during build/startup if this required configuration is missing.
-export const BOARD_ID = process.env.NEXT_PUBLIC_BOARD_ID;
+export const BOARD_ID = process.env.NEXT_PUBLIC_BOARD_ID ?? "";Alternatively, validate at build time or where it's consumed.
📝 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.
Suggested change
| export const BOARD_ID = process.env.NEXT_PUBLIC_BOARD_ID; | |
| export const BOARD_ID = process.env.NEXT_PUBLIC_BOARD_ID ?? ""; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/components/constants.ts` at line 1, The BOARD_ID export can be
undefined when NEXT_PUBLIC_BOARD_ID is not set which leads to invalid fetch URLs
(e.g., in open-roles.tsx); update the export in constants.ts to validate at
module load: check process.env.NEXT_PUBLIC_BOARD_ID and either throw a clear
error if missing or provide a safe fallback value, then export the
validated/normalized BOARD_ID so consumers like open-roles.tsx always receive a
defined string (reference symbol: BOARD_ID).
Comment on lines
+37
to
+41
| if (orm.includes(pathname.split("?")[0])) { | ||
| return "orm"; | ||
| } | ||
| // Add more conditions here for other pages as needed | ||
| return "ppg"; // default variant |
Contributor
There was a problem hiding this comment.
Path matching is too strict for careers routes.
Exact matching against "/careers" will miss cases like "/careers/" and nested routes, which can silently fall back to "ppg".
Proposed fix
-const orm = ["/careers"];
+const orm = ["/careers"];
export function NavigationWrapper({ links, utm }: NavigationWrapperProps) {
const pathname = usePathname();
// Determine button variant based on pathname
const getButtonVariant = (): "ppg" | "orm" | "default-stronger" => {
- if (orm.includes(pathname.split("?")[0])) {
+ const normalizedPath = pathname.replace(/\/+$/, "") || "/";
+ if (orm.some((p) => normalizedPath === p || normalizedPath.startsWith(`${p}/`))) {
return "orm";
}📝 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.
Suggested change
| if (orm.includes(pathname.split("?")[0])) { | |
| return "orm"; | |
| } | |
| // Add more conditions here for other pages as needed | |
| return "ppg"; // default variant | |
| const normalizedPath = pathname.replace(/\/+$/, "") || "/"; | |
| if (orm.some((p) => normalizedPath === p || normalizedPath.startsWith(`${p}/`))) { | |
| return "orm"; | |
| } | |
| // Add more conditions here for other pages as needed | |
| return "ppg"; // default variant |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/components/navigation-wrapper.tsx` around lines 37 - 41, The
current exact path check in navigation-wrapper.tsx is too strict and misses
"/careers/", "/careers/anything" etc.; update the logic that uses
orm.includes(pathname.split("?")[0]) to normalize the path (strip query and
trailing slash) and perform prefix matching: e.g., compute const pathnameNoQuery
= pathname.split("?")[0].replace(/\/+$/,'') and then use orm.some(p =>
pathnameNoQuery === p || pathnameNoQuery.startsWith(p + "/")) (or specifically
check pathnameNoQuery.startsWith("/careers")) so careers and nested routes map
to the "orm" variant instead of falling back to "ppg".
Comment on lines
+16
to
+17
| export class ResponsiveMasonry extends React.Component<ResponsiveMasonryProps> {} | ||
| export default class Masonry extends React.Component<MasonryProps> {} |
Contributor
There was a problem hiding this comment.
React is referenced but not imported in the module declaration.
The type declaration uses React.Component but React isn't in scope. In module declarations, you need to either import React or use the global namespace.
🔧 Fix the React reference
declare module "react-responsive-masonry" {
- import { ReactNode } from "react";
+ import React, { ReactNode } from "react";
export interface ResponsiveMasonryProps {
columnsCountBreakPoints?: Record<number, number>;
children?: ReactNode;
}
export interface MasonryProps {
columnsCount?: number;
gutter?: string;
children?: ReactNode;
center?: boolean;
}
export class ResponsiveMasonry extends React.Component<ResponsiveMasonryProps> {}
export default class Masonry extends React.Component<MasonryProps> {}
}📝 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.
Suggested change
| export class ResponsiveMasonry extends React.Component<ResponsiveMasonryProps> {} | |
| export default class Masonry extends React.Component<MasonryProps> {} | |
| declare module "react-responsive-masonry" { | |
| import React, { ReactNode } from "react"; | |
| export interface ResponsiveMasonryProps { | |
| columnsCountBreakPoints?: Record<number, number>; | |
| children?: ReactNode; | |
| } | |
| export interface MasonryProps { | |
| columnsCount?: number; | |
| gutter?: string; | |
| children?: ReactNode; | |
| center?: boolean; | |
| } | |
| export class ResponsiveMasonry extends React.Component<ResponsiveMasonryProps> {} | |
| export default class Masonry extends React.Component<MasonryProps> {} | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/types/react-responsive-masonry.d.ts` around lines 16 - 17, The
declaration file references React.Component but never imports React; add an
explicit import of React (e.g. import * as React from 'react') at the top of the
module declaration so React is in scope, then keep the exported classes
(ResponsiveMasonry and Masonry) as extending React.Component; this ensures the
symbols ResponsiveMasonry and Masonry compile correctly in the declaration file.
Comment on lines
+39
to
+53
| <div className="title flex items-center"> | ||
| {author.title && ( | ||
| <span className="font-[600] text-2xs uppercase"> | ||
| {author.title} | ||
| </span> | ||
| )} | ||
| {author.company && ( | ||
| <> | ||
| <Separator orientation="vertical" className="mx-1 h-3.5" /> | ||
| <span className="font-[400] text-xs uppercase text-foreground-ppg"> | ||
| {author.company} | ||
| </span> | ||
| </> | ||
| )} | ||
| </div> |
Contributor
There was a problem hiding this comment.
Edge case: Separator renders with no preceding content when company exists but title doesn't.
If author.title is undefined/empty but author.company is provided, the Separator will appear at the start of the div with nothing before it, which may look visually awkward.
🔧 Proposed fix to only show Separator when both exist
{author.title && (
<span className="font-[600] text-2xs uppercase">
{author.title}
</span>
)}
{author.company && (
<>
- <Separator orientation="vertical" className="mx-1 h-3.5" />
+ {author.title && (
+ <Separator orientation="vertical" className="mx-1 h-3.5" />
+ )}
<span className="font-[400] text-xs uppercase text-foreground-ppg">
{author.company}
</span>
</>
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/quote.tsx` around lines 39 - 53, The Separator can
render at the start when author.title is falsy but author.company exists; update
the JSX in the quote component so the Separator is only rendered when both
author.title and author.company are present (i.e., wrap Separator in a
conditional that checks both), leaving the existing title and company spans
unchanged; reference the author.title, author.company checks and the Separator
element in the component to locate and adjust the rendering logic.
Contributor
|
Hi @carlagn Cuuld we try to have same height for the testimonials boxes ? I found this visual a bit "old" ... but just a nit honestly as there nothing maybe to about it right now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements