fix(FLEETMDM-010-2): 2 review findings across 2 files - #1887
fix(FLEETMDM-010-2): 2 review findings across 2 files#1887flamingo[bot] wants to merge 2 commits into
Conversation
![flamingo[bot]](https://avatars.githubusercontent.com/in/3426709?s=60&v=4){kind=small}
| @@ -21,11 +23,11 @@ export function ClickUpIcon({ | |||
| className={className} | |||
There was a problem hiding this comment.
🦩 🟠 ClickUpIcon hardcodes SVG gradient IDs (clickup-grad-1/2) causing DOM collisions when rendered multiple times
In ClickUpIcon (openframe-frontend-core/src/components/icons/clickup-icon.tsx), replaced hardcoded gradient ids 'clickup-grad-1'/'clickup-grad-2' with unique ids generated via React.useId() (gradId1, gradId2), and updated the linearGradient id attributes plus the fill="url(#...)" references on both path elements to use these instance-unique ids, matching the ElestioLogo convention and preventing DOM id collisions when the icon is rendered multiple times.
🤖 Prompt for AI agents
In openframe-frontend-core/src/components/icons/clickup-icon.tsx around line 21, review and complete this code-review fix: ClickUpIcon hardcodes SVG gradient IDs (clickup-grad-1/2) causing DOM collisions when rendered multiple times.
What the draft fix changed: In ClickUpIcon (openframe-frontend-core/src/components/icons/clickup-icon.tsx), replaced hardcoded gradient ids 'clickup-grad-1'/'clickup-grad-2' with unique ids generated via React.useId() (gradId1, gradId2), and updated the linearGradient id attributes plus the fill="url(#...)" references on both path elements to use these instance-unique ids, matching the ElestioLogo convention and preventing DOM id collisions when the icon is rendered multiple times.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer
| import React, { useId } from 'react'; | ||
|
|
||
| interface EmptyVendorIconProps { | ||
| className?: string; | ||
| } | ||
|
|
||
| /** @deprecated Use icons from icons-v2-generated instead. */ | ||
| export function EmptyVendorIcon({ className }: EmptyVendorIconProps) { | ||
| const uid = useId(); | ||
| const patternId = `pattern0_1643_33816_${uid}`; | ||
| const imageId = `image0_1643_33816_${uid}`; | ||
| return ( | ||
| <svg | ||
| width="24" |
There was a problem hiding this comment.
🦩 🟠 EmptyVendorIcon hardcodes pattern/image DOM ids causing collisions on repeated renders
In EmptyVendorIcon (empty-vendor-icon.tsx), replaced the hardcoded static SVG ids pattern0_1643_33816 and image0_1643_33816 with per-instance unique ids generated via React's useId() hook (patternId, imageId), and updated the fill="url(#...)", <pattern id=...>, xlinkHref="#...", and <image id=...> references accordingly. This ensures each rendered instance of the component gets distinct DOM ids, resolving the collision when multiple instances are rendered on the same page.
🤖 Prompt for AI agents
In openframe-frontend-core/src/components/icons/empty-vendor-icon.tsx around line 14, review and complete this code-review fix: EmptyVendorIcon hardcodes pattern/image DOM ids causing collisions on repeated renders.
What the draft fix changed: In EmptyVendorIcon (empty-vendor-icon.tsx), replaced the hardcoded static SVG ids `pattern0_1643_33816` and `image0_1643_33816` with per-instance unique ids generated via React's `useId()` hook (`patternId`, `imageId`), and updated the `fill="url(#...)"`, `<pattern id=...>`, `xlinkHref="#..."`, and `<image id=...>` references accordingly. This ensures each rendered instance of the component gets distinct DOM ids, resolving the collision when multiple instances are rendered on the same page.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer
Closes 2 review findings across 2 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
openframe-frontend-core/src/components/icons/clickup-icon.tsx:21openframe-frontend-core/src/components/icons/empty-vendor-icon.tsx:14What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
59051284-b3d6-4391-892c-38c8737386e1Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.