Merged
Conversation
✅ Deploy Preview for antenna-ssec ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for antenna-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughComprehensive migration from custom design-system components to Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/src/data-services/models/pipeline.ts (1)
63-71:⚠️ Potential issue | 🟠 MajorBug:
versionLabeldisplays string length instead of version name.Line 69 uses
this._pipeline.version_name?.lengthinside the template string, which will display the length of the version name (a number) rather than the actual version name string.🐛 Proposed fix
get versionLabel(): string | undefined { if (this._pipeline.version == undefined) { return undefined } return this._pipeline.version_name?.length - ? `${this._pipeline.version} "${this._pipeline.version_name?.length}"` + ? `${this._pipeline.version} "${this._pipeline.version_name}"` : `${this._pipeline.version}` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/data-services/models/pipeline.ts` around lines 63 - 71, The versionLabel getter is accidentally interpolating the length of the version name instead of the version name itself; update the getter (versionLabel) so that when this._pipeline.version_name exists you return `${this._pipeline.version} "${this._pipeline.version_name}"` (not using .length), otherwise return `${this._pipeline.version}`; ensure you still handle undefined this._pipeline.version by returning undefined as currently implemented.ui/src/components/terms-of-service-info/terms-of-service-info.tsx (1)
12-17:⚠️ Potential issue | 🟡 MinorStale closure in cleanup effect.
The cleanup function references
userPreferencesbut it's not included in the dependency array. This means ifuserPreferenceschanges before unmount, the cleanup will spread the stale object.🛠️ Proposed fix
useEffect(() => { return () => { // Mark message as seen when component unmounts setUserPreferences({ ...userPreferences, termsMessageSeen: true }) } -}, []) +}, [userPreferences, setUserPreferences])Alternatively, use a functional update if
setUserPreferencessupports it:useEffect(() => { return () => { // Mark message as seen when component unmounts - setUserPreferences({ ...userPreferences, termsMessageSeen: true }) + setUserPreferences((prev) => ({ ...prev, termsMessageSeen: true })) } }, [setUserPreferences])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/terms-of-service-info/terms-of-service-info.tsx` around lines 12 - 17, The cleanup effect in useEffect references userPreferences but doesn't list it as a dependency, causing a stale closure; update the cleanup to use a functional state update so it doesn't depend on the closed-over userPreferences (e.g., call setUserPreferences(prev => ({ ...prev, termsMessageSeen: true })) inside the cleanup of the useEffect in terms-of-service-info.tsx), and ensure setUserPreferences is stable or included in the dependency array if it's not memoized.
🟡 Minor comments (11)
ui/src/utils/constants.ts-117-119 (1)
117-119:⚠️ Potential issue | 🟡 MinorUpdate both URLs to use consistent domain format (with
www).
LANDING_PAGE_URLusesinsectai.orgwhileLANDING_PAGE_CONTACT_URLuseswww.insectai.org. The non-www domain redirects (301) to the www version, so both constants should use the www format for consistency and to avoid unnecessary redirects.🔧 Suggested fix
-export const LANDING_PAGE_URL = 'https://insectai.org/' +export const LANDING_PAGE_URL = 'https://www.insectai.org/'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/utils/constants.ts` around lines 117 - 119, Update the inconsistent domain in the exported constants so both use the www form: change LANDING_PAGE_URL to 'https://www.insectai.org/' to match LANDING_PAGE_CONTACT_URL; locate the constants LANDING_PAGE_URL and LANDING_PAGE_CONTACT_URL in the file and make the LANDING_PAGE_URL value use the www subdomain to avoid the 301 redirect and keep formats consistent.ui/src/pages/deployment-details/deployment-details-form/section-location/section-location.tsx-119-124 (1)
119-124:⚠️ Potential issue | 🟡 MinorAdd explicit
typeattribute to form navigation buttons.Both buttons lack a
typeattribute. In HTML forms, buttons default totype="submit", causing the "Back" button to trigger form submission before callingonBack. Addtype="button"to prevent unintended submission:Suggested fix
- <Button onClick={onBack} size="small" variant="outline"> + <Button onClick={onBack} size="small" variant="outline" type="button"> <span>{translate(STRING.BACK)}</span> </Button> - <Button onClick={onNext} size="small" variant="success"> + <Button onClick={onNext} size="small" variant="success" type="button"> <span>{translate(STRING.NEXT)}</span> </Button>The same pattern exists in
section-source-images.tsx. Codebase convention shows explicittype="button"for non-submit buttons (e.g.,deployment-details-form.tsxCancel button,image-upload.tsx).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/deployment-details/deployment-details-form/section-location/section-location.tsx` around lines 119 - 124, The Back and Next navigation Buttons in section-location.tsx (handlers onBack and onNext) are missing an explicit type and therefore default to type="submit"; update both Button usages to include type="button" to prevent unintended form submission when clicking Back (and for consistency for Next if it should not submit). Also apply the same change to the corresponding Buttons in section-source-images.tsx, following the existing convention used for non-submit buttons like the Cancel button in deployment-details-form.tsx and the button in image-upload.tsx.ui/src/design-system/components/table/basic-table-cell/basic-table-cell.module.scss-11-11 (1)
11-11:⚠️ Potential issue | 🟡 MinorUse
overflow-wrap: break-wordinstead of the deprecatedword-break: break-word.The value
break-wordfor theword-breakproperty is deprecated. The CSS standard recommends usingoverflow-wrap: break-wordfor this behavior.Proposed fix
.label { display: block; `@include` paragraph-medium(); color: $color-neutral-700; - word-break: break-word; + overflow-wrap: break-word; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/design-system/components/table/basic-table-cell/basic-table-cell.module.scss` at line 11, Replace the deprecated CSS declaration "word-break: break-word" in basic-table-cell.module.scss with the modern equivalent by removing or replacing that property and adding "overflow-wrap: break-word" on the same selector (the basic table cell rule) so long words wrap correctly; update any related rules that rely on word-break in the same module to use overflow-wrap instead to maintain behavior across browsers.ui/src/data-services/models/occurrence.ts-23-25 (1)
23-25:⚠️ Potential issue | 🟡 Minor
createdAtshould not return an invalidDateinstance on bad/missing data.
Line 24can produceInvalid Datewhile the getter is typed as always validDate. Please align with a guardedDate | undefinedreturn (and validate parse result).Suggested fix
- get createdAt(): Date { - return new Date(this._occurrence.created_at) + get createdAt(): Date | undefined { + if (!this._occurrence.created_at) { + return undefined + } + const date = new Date(this._occurrence.created_at) + return Number.isNaN(date.getTime()) ? undefined : date } @@ get updatedAt(): Date | undefined { if (!this._occurrence.updated_at) { return undefined } - - return new Date(this._occurrence.updated_at) + const date = new Date(this._occurrence.updated_at) + return Number.isNaN(date.getTime()) ? undefined : date }Also applies to: 27-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/data-services/models/occurrence.ts` around lines 23 - 25, The createdAt getter currently returns new Date(this._occurrence.created_at) which can yield an invalid Date; change the getter signature to return Date | undefined, validate input (ensure this._occurrence.created_at exists and that constructed Date is valid via !isNaN(date.getTime())), and return undefined for missing/invalid values; apply the same pattern to the other similar getters (e.g., updatedAt/resolvedAt or any getters around lines 27-33) and update any callers to handle the optional Date return.ui/src/data-services/models/entity.ts-21-27 (1)
21-27:⚠️ Potential issue | 🟡 MinorGuard against
Invalid Datevalues before exposing timestamps.
Line 26andLine 46return aDateeven when parsing fails, which can propagate"Invalid Date"into table cells.Suggested fix
get createdAt(): Date | undefined { if (!this._data.created_at) { return undefined } - - return new Date(this._data.created_at) + const date = new Date(this._data.created_at) + return Number.isNaN(date.getTime()) ? undefined : date } @@ get updatedAt(): Date | undefined { if (!this._data.updated_at) { return undefined } - - return new Date(this._data.updated_at) + const date = new Date(this._data.updated_at) + return Number.isNaN(date.getTime()) ? undefined : date }Also applies to: 41-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/data-services/models/entity.ts` around lines 21 - 27, The createdAt (and similarly updatedAt) getters currently return new Date(this._data.created_at) even when parsing fails; update get createdAt and get updatedAt to parse the timestamp into a Date, test validity via isNaN(date.getTime()) (or date instanceof Date && !isNaN(...)) and return undefined when invalid, otherwise return the Date; reference the getters named createdAt and updatedAt and the backing field this._data.created_at / this._data.updated_at to locate the changes.ui/src/data-services/models/species.ts-39-41 (1)
39-41:⚠️ Potential issue | 🟡 MinorApply the same invalid-date guard to species timestamps.
Line 40andLine 113can returnInvalid Datewhen backend values are malformed, which leaks to UI formatting.Suggested fix
- get createdAt(): Date { - return new Date(this._species.created_at) + get createdAt(): Date | undefined { + if (!this._species.created_at) { + return undefined + } + const date = new Date(this._species.created_at) + return Number.isNaN(date.getTime()) ? undefined : date } @@ get updatedAt(): Date | undefined { if (!this._species.updated_at) { return undefined } - return new Date(this._species.updated_at) + const date = new Date(this._species.updated_at) + return Number.isNaN(date.getTime()) ? undefined : date }Also applies to: 109-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/data-services/models/species.ts` around lines 39 - 41, The createdAt getter (returning new Date(this._species.created_at)) and the other timestamp getters around lines 109-114 should guard against malformed backend values; parse the date into a Date object, check isNaN(date.getTime()) and if invalid return null (or undefined) instead of the Date object so the UI won't receive "Invalid Date". Update the createdAt getter and the corresponding timestamp getters (e.g., updatedAt/observedAt that reference this._species.*_at) to perform this validation and return a safe null/undefined when the parsed date is invalid.ui/src/pages/auth/login.tsx-96-103 (1)
96-103:⚠️ Potential issue | 🟡 MinorHardcoded copy here bypasses localization.
Line 96andLine 101-103introduce raw English strings, which breaks consistency for translated locales. Please move both strings toSTRINGkeys and render viatranslate(...).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/auth/login.tsx` around lines 96 - 103, The span text "Get in touch" and the trailing sentence "Please reach out and we will help you." are hardcoded; add two new keys to the STRING enum/lookup (e.g., GET_IN_TOUCH and FORGOT_PASSWORD_HELP), add their localized values, and replace the raw strings in the JSX with translate(STRING.GET_IN_TOUCH) for the <span> and translate(STRING.FORGOT_PASSWORD_HELP) for the paragraph (instead of the current inline English text after translate(STRING.FORGOT_PASSWORD)). Ensure you update any i18n resource files so translations are available.ui/src/pages/auth/sign-up.tsx-79-82 (1)
79-82:⚠️ Potential issue | 🟡 MinorRemove unused
loadingprop from Button.The
loading={isLoading}prop is not processed by nova-ui-kit's Button component (it doesn't support built-in loading state), making it unnecessary. The manualLoader2Iconrender is the correct approach and should be retained. Remove the unused prop to match the pattern used elsewhere in the codebase (e.g.,taxon-tags/tags-form.tsx).🔍 Proposed fix
-<Button type="submit" variant="success" loading={isLoading}> +<Button type="submit" variant="success"> <span>{translate(STRING.SIGN_UP)}</span> {isLoading ? <Loader2Icon className="w-4 h-4 animate-spin" /> : null} </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/auth/sign-up.tsx` around lines 79 - 82, The Button in the sign-up form is being passed an unsupported loading prop; remove the unused loading={isLoading} from the Button element in sign-up.tsx and keep the existing manual spinner render (the Loader2Icon conditional) so the submit button uses <Button type="submit" variant="success"> with the inner <span>{translate(STRING.SIGN_UP)}</span> and the existing {isLoading ? <Loader2Icon ... /> : null} logic unchanged.ui/src/data-services/models/deployment.ts-37-42 (1)
37-42:⚠️ Potential issue | 🟡 MinorType assertion may cause runtime error if
updatedAtis undefined.The
as Dateassertion assumesupdatedAtis always defined, but if any job in_jobshas an undefinedupdatedAt, callinggetTime()will throw a runtime error.Additionally,
Array.sort()mutates the original_jobsarray. If this is unintended, consider usingtoSorted()or spreading before sorting.🔧 Proposed fix with null safety
- return this._jobs.sort((j1: Job, j2: Job) => { - const date1 = j1.updatedAt as Date - const date2 = j2.updatedAt as Date + return [...this._jobs].sort((j1: Job, j2: Job) => { + const date1 = j1.updatedAt + const date2 = j2.updatedAt - return date2.getTime() - date1.getTime() + return (date2?.getTime() ?? 0) - (date1?.getTime() ?? 0) })[0]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/data-services/models/deployment.ts` around lines 37 - 42, The sorting logic on this._jobs assumes each Job.updatedAt is a Date and mutates the array; update the code that computes the latest job so it first works on a copy (e.g., spread or toSorted) to avoid mutating this._jobs and guard against undefined updatedAt by using safe null checks or fallbacks (e.g., compute timestamps with (job.updatedAt?.getTime() ?? 0) or filter out jobs without updatedAt) before comparing; ensure the method using Job and updatedAt returns undefined or handles empty arrays if there are no valid timestamps.ui/src/pages/project/pipelines/pipelines-columns.tsx-40-43 (1)
40-43:⚠️ Potential issue | 🟡 MinorLong unbroken description text can still overflow.
whiteSpace: 'normal'won’t break long tokens. AddoverflowWrap(orwordBreak) so very long strings don’t blow out table layout.💡 Suggested tweak
style={{ width: '320px', whiteSpace: 'normal', + overflowWrap: 'anywhere', }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/project/pipelines/pipelines-columns.tsx` around lines 40 - 43, The inline style object currently set as style={{ width: '320px', whiteSpace: 'normal' }} can still allow long unbroken tokens to overflow; update that style in pipelines-columns.tsx (the same style object) to include overflowWrap: 'anywhere' or overflowWrap: 'break-word' (or alternatively wordBreak: 'break-word' / wordBreak: 'break-all' for more aggressive breaking) so very long strings wrap instead of blowing out the column layout.ui/src/design-system/components/wizard/status-bullet/status-bullet.tsx-30-30 (1)
30-30:⚠️ Potential issue | 🟡 MinorDon’t hide
0when renderingvalue.Using a truthy check drops valid numeric
0values.Suggested fix
- {value ? <span>{value}</span> : null} + {value != null ? <span>{value}</span> : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/design-system/components/wizard/status-bullet/status-bullet.tsx` at line 30, The current conditional in status-bullet.tsx uses a truthy check on value which hides valid numeric 0; change the check to explicitly test for null/undefined (e.g., use value != null) before rendering the <span> so that 0 is rendered. Locate the conditional that references value in the StatusBullet component and replace the truthy check with an explicit null/undefined guard.
🧹 Nitpick comments (5)
ui/src/components/cookie-dialog/cookie-dialog.module.scss (1)
48-64: Consider removing redundant mobile override.After setting the base
.actionsgap to8px(line 50), the mobile media query override (lines 62-64) is now redundant since both specify the same value. You could remove the.actionsblock from the media query for cleaner CSS.🧹 Proposed cleanup
`@media` only screen and (max-width: $small-screen-breakpoint) { .dialog { left: 0; bottom: 0; max-width: 100%; padding: 16px; border-radius: 0; } - - .actions { - gap: 8px; - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/cookie-dialog/cookie-dialog.module.scss` around lines 48 - 64, Remove the redundant `.actions` rule inside the media query in cookie-dialog.module.scss: the base `.actions { gap: 8px; }` already applies the desired spacing, so delete the `.actions { gap: 8px; }` block within the `@media only screen and (max-width: $small-screen-breakpoint)` section to keep the CSS concise while leaving the `.dialog` overrides intact.ui/src/pages/deployment-details/deployment-details-form/deployment-details-form.tsx (1)
126-134: Set explicittype="button"on Save action for safety.This action is click-driven (
onClick), so settingtype="button"avoids implicit submit behavior if surrounding markup changes later.Suggested fix
<Button + type="button" disabled={!allValid || !someDirty || isLoading} onClick={onSaveClick} size="small" variant="success" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/deployment-details/deployment-details-form/deployment-details-form.tsx` around lines 126 - 134, The Save Button currently relies on onClick (onSaveClick) but lacks an explicit type, which can cause an implicit form submit if markup changes; update the Button JSX (the element using props disabled={!allValid || !someDirty || isLoading}, onClick={onSaveClick}, size="small", variant="success") to include type="button" so the click only triggers onSaveClick and never accidentally submits a surrounding form.ui/src/pages/occurrence-details/id-quick-actions/id-button.tsx (1)
38-60: Consider disabling button during loading to prevent duplicate submissions.The button is disabled only when
isSuccessbut not whenisLoading. This allows users to click repeatedly during the loading state, potentially triggering multiplecreateIdentificationscalls. If the previous implementation'sloadingprop also disabled the button, this could be a behavioral regression.🔧 Proposed fix
<Button className={classNames('justify-between', { 'text-destructive': error })} - disabled={isSuccess} + disabled={isLoading || isSuccess} onClick={() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/occurrence-details/id-quick-actions/id-button.tsx` around lines 38 - 60, The Button is only disabled when isSuccess, so users can click while isLoading and trigger duplicate createIdentifications calls; update the disabled logic on the Button (the JSX using Button, classNames, and the onClick that calls addRecentIdentification and createIdentifications) to disable the control when either isLoading or isSuccess is true (e.g., disabled={isSuccess || isLoading}) so the button is inert during the loading state.ui/src/pages/deployment-details/deployment-details-form/section-location/location-map/location-map.tsx (1)
29-43: Consider disabling button while loading location.The button allows repeated clicks while
loadingLocationis true, which could trigger multiplemapRef.current.locate()calls. If the previous design-system button had aloadingprop that also disabled the button, this might be a behavioral regression.🔧 Proposed fix to prevent multiple clicks
<Button + disabled={loadingLocation} onClick={() => { if (mapRef.current) { mapRef.current.locate() setLoadingLocation(true) } }} size="small" variant="outline" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/deployment-details/deployment-details-form/section-location/location-map/location-map.tsx` around lines 29 - 43, The Button currently still accepts clicks while loadingLocation is true which lets mapRef.current.locate() be called multiple times; update the Button usage in location-map.tsx (the Button component where onClick calls mapRef.current.locate() and setLoadingLocation(true)) to prevent repeated clicks by passing the appropriate prop when loadingLocation is true (prefer the Button's loading prop if it exists, otherwise set disabled={loadingLocation}); ensure the UI still shows the Loader2Icon when loadingLocation is true and that onClick exits early if mapRef.current is missing.ui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsx (1)
25-37: Button should be disabled during loading to prevent duplicate sync operations.The button is not disabled while
isLoadingis true, allowing users to potentially trigger multiple sync operations by clicking repeatedly. This could cause unintended duplicate syncs.🔧 Proposed fix
<Button - disabled={!isConnected || isSuccess} + disabled={!isConnected || isLoading || isSuccess} onClick={() => syncDeploymentSourceImages(deploymentId)} size="small" variant="success" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsx` around lines 25 - 37, The Sync button can be clicked while a sync is in progress; update the Button in sync-source-images.tsx (the JSX using <Button ... onClick={() => syncDeploymentSourceImages(deploymentId)} ...> and the isLoading / isSuccess / isConnected flags) to also disable when isLoading is true (i.e., include isLoading in the disabled condition) so users can't trigger duplicate syncs; optionally guard the click handler to no-op when isLoading to be extra safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/src/data-services/models/capture-details.ts`:
- Around line 28-31: The comparator in currentJob unsafely casts j1.updatedAt
and j2.updatedAt to Date and calls getTime(), which can throw if updatedAt is
undefined; update the comparator used to sort currentJob so it safely handles
undefined updatedAt (e.g., compute timestamps via a guarded expression or
nullish-coalescing fallback when reading j1.updatedAt and j2.updatedAt before
calling getTime()), and apply the same defensive fix to the equivalent
comparators in capture-set.ts and deployment.ts; reference the updatedAt
property and the comparator used in currentJob to locate and patch the code.
In `@ui/src/data-services/models/capture-set.ts`:
- Around line 32-35: The comparator uses unsafe casts (j1.updatedAt as Date /
j2.updatedAt as Date) and calls getTime() which will throw if updatedAt is
undefined; change the comparator to safely get timestamps using optional
chaining and a fallback (e.g., const t1 = j1.updatedAt?.getTime() ?? 0 and const
t2 = j2.updatedAt?.getTime() ?? 0) and return t2 - t1; apply the same fix
pattern to the analogous usages in deployment.ts and capture-details.ts (look
for the comparator or any direct getTime() calls on updatedAt).
In `@ui/src/data-services/models/session.ts`:
- Around line 103-105: The createdAt getter currently returns the wrong
timestamp; update the get createdAt() accessor to return this._event.created_at
instead of this._event.updated_at so it reflects the session creation time;
locate the createdAt getter on the Session model (the get createdAt() method
that references this._event) and change the returned field to created_at.
In `@ui/src/design-system/components/image-carousel/image-carousel.tsx`:
- Around line 174-180: The carousel's icon-only navigation buttons (the Button
elements that call showPrev(slideIndex) and showNext(slideIndex) and render
ChevronLeftIcon/ChevronRightIcon) lack accessible names; add an accessible label
to each Button (e.g., aria-label="Previous slide" and aria-label="Next slide" or
include visually hidden text) so screen readers can distinguish the controls
while keeping the icons for sighted users.
In `@ui/src/design-system/components/pagination-bar/pagination-bar.tsx`:
- Around line 41-48: The icon-only pagination Buttons in PaginationBar render
without accessible names; update the Button components (the one with
<ChevronLeft /> and the one with <ChevronRight />) to include descriptive
accessible labels (e.g., aria-label or aria-labelledby like "Previous page" and
"Next page") so assistive tech can announce their purpose, keeping the existing
disabled logic (disabled={currentPage <= firstPage} / disabled={currentPage >=
lastPage}) and onClick handlers (setPage(currentPage - 1) / setPage(currentPage
+ 1}) intact.
In `@ui/src/design-system/components/popover/popover.module.scss`:
- Line 9: The popover's z-index is set to 50 in popover.module.scss which
conflicts with the dialog/modal and page-footer stacks; lower the popover
stacking context to a value below the modal (e.g., change the popover z-index
from 50 to 40 in the popover z-index rule), then ensure the dialog/modal overlay
and content (the dialog component) and page-footer use higher, distinct z-index
values (e.g., 60+ for modal overlay/content and 50 for footer) so modals always
layer above popovers.
In `@ui/src/pages/auth/reset-password.tsx`:
- Around line 72-77: The submit Button in reset-password.tsx (component Button
rendering the submit with {translate(STRING.SEND_INSTRUCTIONS)} and Loader2Icon)
is not disabled during isLoading, allowing duplicate submissions; update the
Button props to set disabled={isLoading} (and/or add aria-disabled when
appropriate) so the button becomes non-interactive while isLoading is true to
prevent repeated reset requests and preserve the existing submit behavior.
In
`@ui/src/pages/captures/upload-images-dialog/select-images-section/select-images-section.tsx`:
- Around line 49-59: The icon-only Button rendering the XIcon (and the other
icon-only Button in the same component) lacks an accessible name and an explicit
button type; update the Button elements in select-images-section.tsx (the ones
using XIcon and the other icon-only control) to include type="button" and an
accessible name (e.g., aria-label="Remove image" or aria-label appropriate for
the action) so screen readers can identify them; optionally add a title
attribute for hover tooltips, and keep the existing onClick handler
(setImages(images.filter(...))) unchanged.
In
`@ui/src/pages/deployment-details/deployment-details-form/section-general/section-general.tsx`:
- Around line 130-132: The Button inside the form lacks an explicit type which
can cause accidental form submissions; update the Button element used with
onClick={onNext} (the Button rendering the
<span>{translate(STRING.NEXT)}</span>) to include type="button" so the click
invokes onNext without submitting the enclosing form, matching other components
like processing-form.tsx.
In `@ui/src/pages/job-details/job-actions/cancel-job.tsx`:
- Around line 11-12: The cancel button currently only disables when isSuccess,
allowing multiple rapid clicks to call cancelJob(jobId) while isLoading is true;
update the button to also disable during loading (e.g., disabled={isSuccess ||
isLoading}) or add a guard inside the onClick to no-op when isLoading is true so
cancelJob(jobId) cannot be invoked while the request is pending (refer to the
isLoading flag and the cancelJob function used in the onClick handler).
In `@ui/src/pages/job-details/job-actions/queue-job.tsx`:
- Around line 11-12: The button currently only disables when isSuccess, allowing
repeated clicks while the request is in progress; change the disable logic to
include the loading state (use isLoading || isSuccess) and/or guard the click
handler by returning early if isLoading is true. Update the Button's disabled
prop and the onClick wrapper around queueJob(jobId) (referencing queueJob,
isLoading, isSuccess) so the action cannot be invoked while the call is in
flight.
In `@ui/src/pages/job-details/job-actions/retry-job.tsx`:
- Around line 10-21: The Retry button allows duplicate requests because it only
disables when isSuccess; update the click guard to also prevent clicks while
isLoading by disabling the Button when isLoading (e.g., disabled={isSuccess ||
isLoading}) and/or make the onClick handler no-op when isLoading (wrap
retryJob(jobId) behind a guard using isLoading). Modify the UI that renders
Button (the Button component and its onClick) so retryJob is not called while
isLoading to avoid duplicate retry mutations.
In `@ui/src/pages/occurrence-details/id-quick-actions/id-quick-actions.tsx`:
- Around line 71-77: The dynamic Tailwind interpolation using className={zIndex
? `z-${zIndex}` : undefined} will not be compiled by Tailwind; instead remove
the runtime class construction and apply the stacking context via an inline
style on the same element (use the existing zIndex variable to set style={{
zIndex: Number(zIndex) }} or style={{ zIndex }} ensuring correct type), keep any
other static className values separate, and remove the `z-${...}` usage so the
component (the element receiving the className prop in id-quick-actions.tsx)
uses inline styles for z-index.
In
`@ui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsx`:
- Around line 89-96: The prev/next icon-only Buttons (the Button components
wrapping ChevronLeftIcon and ChevronRightIcon used with goToPrev and goToNext)
lack accessible names; add aria-label attributes (e.g., aria-label="Previous
capture" on the Button that checks activeCapture?.prevCaptureId and
aria-label="Next capture" on the Button that checks
activeCapture?.nextCaptureId) so screen readers announce the action even when
the button is icon-only; keep the existing disabled props and handlers
unchanged.
---
Outside diff comments:
In `@ui/src/components/terms-of-service-info/terms-of-service-info.tsx`:
- Around line 12-17: The cleanup effect in useEffect references userPreferences
but doesn't list it as a dependency, causing a stale closure; update the cleanup
to use a functional state update so it doesn't depend on the closed-over
userPreferences (e.g., call setUserPreferences(prev => ({ ...prev,
termsMessageSeen: true })) inside the cleanup of the useEffect in
terms-of-service-info.tsx), and ensure setUserPreferences is stable or included
in the dependency array if it's not memoized.
In `@ui/src/data-services/models/pipeline.ts`:
- Around line 63-71: The versionLabel getter is accidentally interpolating the
length of the version name instead of the version name itself; update the getter
(versionLabel) so that when this._pipeline.version_name exists you return
`${this._pipeline.version} "${this._pipeline.version_name}"` (not using
.length), otherwise return `${this._pipeline.version}`; ensure you still handle
undefined this._pipeline.version by returning undefined as currently
implemented.
---
Minor comments:
In `@ui/src/data-services/models/deployment.ts`:
- Around line 37-42: The sorting logic on this._jobs assumes each Job.updatedAt
is a Date and mutates the array; update the code that computes the latest job so
it first works on a copy (e.g., spread or toSorted) to avoid mutating this._jobs
and guard against undefined updatedAt by using safe null checks or fallbacks
(e.g., compute timestamps with (job.updatedAt?.getTime() ?? 0) or filter out
jobs without updatedAt) before comparing; ensure the method using Job and
updatedAt returns undefined or handles empty arrays if there are no valid
timestamps.
In `@ui/src/data-services/models/entity.ts`:
- Around line 21-27: The createdAt (and similarly updatedAt) getters currently
return new Date(this._data.created_at) even when parsing fails; update get
createdAt and get updatedAt to parse the timestamp into a Date, test validity
via isNaN(date.getTime()) (or date instanceof Date && !isNaN(...)) and return
undefined when invalid, otherwise return the Date; reference the getters named
createdAt and updatedAt and the backing field this._data.created_at /
this._data.updated_at to locate the changes.
In `@ui/src/data-services/models/occurrence.ts`:
- Around line 23-25: The createdAt getter currently returns new
Date(this._occurrence.created_at) which can yield an invalid Date; change the
getter signature to return Date | undefined, validate input (ensure
this._occurrence.created_at exists and that constructed Date is valid via
!isNaN(date.getTime())), and return undefined for missing/invalid values; apply
the same pattern to the other similar getters (e.g., updatedAt/resolvedAt or any
getters around lines 27-33) and update any callers to handle the optional Date
return.
In `@ui/src/data-services/models/species.ts`:
- Around line 39-41: The createdAt getter (returning new
Date(this._species.created_at)) and the other timestamp getters around lines
109-114 should guard against malformed backend values; parse the date into a
Date object, check isNaN(date.getTime()) and if invalid return null (or
undefined) instead of the Date object so the UI won't receive "Invalid Date".
Update the createdAt getter and the corresponding timestamp getters (e.g.,
updatedAt/observedAt that reference this._species.*_at) to perform this
validation and return a safe null/undefined when the parsed date is invalid.
In
`@ui/src/design-system/components/table/basic-table-cell/basic-table-cell.module.scss`:
- Line 11: Replace the deprecated CSS declaration "word-break: break-word" in
basic-table-cell.module.scss with the modern equivalent by removing or replacing
that property and adding "overflow-wrap: break-word" on the same selector (the
basic table cell rule) so long words wrap correctly; update any related rules
that rely on word-break in the same module to use overflow-wrap instead to
maintain behavior across browsers.
In `@ui/src/design-system/components/wizard/status-bullet/status-bullet.tsx`:
- Line 30: The current conditional in status-bullet.tsx uses a truthy check on
value which hides valid numeric 0; change the check to explicitly test for
null/undefined (e.g., use value != null) before rendering the <span> so that 0
is rendered. Locate the conditional that references value in the StatusBullet
component and replace the truthy check with an explicit null/undefined guard.
In `@ui/src/pages/auth/login.tsx`:
- Around line 96-103: The span text "Get in touch" and the trailing sentence
"Please reach out and we will help you." are hardcoded; add two new keys to the
STRING enum/lookup (e.g., GET_IN_TOUCH and FORGOT_PASSWORD_HELP), add their
localized values, and replace the raw strings in the JSX with
translate(STRING.GET_IN_TOUCH) for the <span> and
translate(STRING.FORGOT_PASSWORD_HELP) for the paragraph (instead of the current
inline English text after translate(STRING.FORGOT_PASSWORD)). Ensure you update
any i18n resource files so translations are available.
In `@ui/src/pages/auth/sign-up.tsx`:
- Around line 79-82: The Button in the sign-up form is being passed an
unsupported loading prop; remove the unused loading={isLoading} from the Button
element in sign-up.tsx and keep the existing manual spinner render (the
Loader2Icon conditional) so the submit button uses <Button type="submit"
variant="success"> with the inner <span>{translate(STRING.SIGN_UP)}</span> and
the existing {isLoading ? <Loader2Icon ... /> : null} logic unchanged.
In
`@ui/src/pages/deployment-details/deployment-details-form/section-location/section-location.tsx`:
- Around line 119-124: The Back and Next navigation Buttons in
section-location.tsx (handlers onBack and onNext) are missing an explicit type
and therefore default to type="submit"; update both Button usages to include
type="button" to prevent unintended form submission when clicking Back (and for
consistency for Next if it should not submit). Also apply the same change to the
corresponding Buttons in section-source-images.tsx, following the existing
convention used for non-submit buttons like the Cancel button in
deployment-details-form.tsx and the button in image-upload.tsx.
In `@ui/src/pages/project/pipelines/pipelines-columns.tsx`:
- Around line 40-43: The inline style object currently set as style={{ width:
'320px', whiteSpace: 'normal' }} can still allow long unbroken tokens to
overflow; update that style in pipelines-columns.tsx (the same style object) to
include overflowWrap: 'anywhere' or overflowWrap: 'break-word' (or alternatively
wordBreak: 'break-word' / wordBreak: 'break-all' for more aggressive breaking)
so very long strings wrap instead of blowing out the column layout.
In `@ui/src/utils/constants.ts`:
- Around line 117-119: Update the inconsistent domain in the exported constants
so both use the www form: change LANDING_PAGE_URL to 'https://www.insectai.org/'
to match LANDING_PAGE_CONTACT_URL; locate the constants LANDING_PAGE_URL and
LANDING_PAGE_CONTACT_URL in the file and make the LANDING_PAGE_URL value use the
www subdomain to avoid the 301 redirect and keep formats consistent.
---
Nitpick comments:
In `@ui/src/components/cookie-dialog/cookie-dialog.module.scss`:
- Around line 48-64: Remove the redundant `.actions` rule inside the media query
in cookie-dialog.module.scss: the base `.actions { gap: 8px; }` already applies
the desired spacing, so delete the `.actions { gap: 8px; }` block within the
`@media only screen and (max-width: $small-screen-breakpoint)` section to keep
the CSS concise while leaving the `.dialog` overrides intact.
In
`@ui/src/pages/deployment-details/deployment-details-form/deployment-details-form.tsx`:
- Around line 126-134: The Save Button currently relies on onClick (onSaveClick)
but lacks an explicit type, which can cause an implicit form submit if markup
changes; update the Button JSX (the element using props disabled={!allValid ||
!someDirty || isLoading}, onClick={onSaveClick}, size="small",
variant="success") to include type="button" so the click only triggers
onSaveClick and never accidentally submits a surrounding form.
In
`@ui/src/pages/deployment-details/deployment-details-form/section-location/location-map/location-map.tsx`:
- Around line 29-43: The Button currently still accepts clicks while
loadingLocation is true which lets mapRef.current.locate() be called multiple
times; update the Button usage in location-map.tsx (the Button component where
onClick calls mapRef.current.locate() and setLoadingLocation(true)) to prevent
repeated clicks by passing the appropriate prop when loadingLocation is true
(prefer the Button's loading prop if it exists, otherwise set
disabled={loadingLocation}); ensure the UI still shows the Loader2Icon when
loadingLocation is true and that onClick exits early if mapRef.current is
missing.
In
`@ui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsx`:
- Around line 25-37: The Sync button can be clicked while a sync is in progress;
update the Button in sync-source-images.tsx (the JSX using <Button ...
onClick={() => syncDeploymentSourceImages(deploymentId)} ...> and the isLoading
/ isSuccess / isConnected flags) to also disable when isLoading is true (i.e.,
include isLoading in the disabled condition) so users can't trigger duplicate
syncs; optionally guard the click handler to no-op when isLoading to be extra
safe.
In `@ui/src/pages/occurrence-details/id-quick-actions/id-button.tsx`:
- Around line 38-60: The Button is only disabled when isSuccess, so users can
click while isLoading and trigger duplicate createIdentifications calls; update
the disabled logic on the Button (the JSX using Button, classNames, and the
onClick that calls addRecentIdentification and createIdentifications) to disable
the control when either isLoading or isSuccess is true (e.g.,
disabled={isSuccess || isLoading}) so the button is inert during the loading
state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aee8d2dc-92ed-40f2-834d-9f3587051e0b
⛔ Files ignored due to path filters (42)
ui/src/design-system/components/icon/assets/checkmark.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/close.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/detections.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/download.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/filters.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/gallery-view.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/identifiers.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/info.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/members.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/photograph.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/play-button.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/check.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/circle-backslash.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/clock.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/cross.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/error.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/external-link.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/heart-filled.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/heart.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/minus.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/options.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/pencil.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/plus.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/question-mark.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/search.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/toggle-down.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/toggle-left.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/toggle-right.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/trash.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/update.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/settings.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/shield-check.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/sort.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/table-view.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/captures.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/deployments.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/jobs.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/occurrences.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/project.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/sessions.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/taxa.svgis excluded by!**/*.svgui/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (98)
ui/package.jsonui/src/components/blueprint-collection/blueprint-collection.tsxui/src/components/cookie-dialog/cookie-dialog.module.scssui/src/components/cookie-dialog/cookie-dialog.tsxui/src/components/filtering/default-filter-control.tsxui/src/components/terms-of-service-info/terms-of-service-info.tsxui/src/data-services/models/algorithm.tsui/src/data-services/models/capture-details.tsui/src/data-services/models/capture-set.tsui/src/data-services/models/deployment.tsui/src/data-services/models/entity.tsui/src/data-services/models/job.tsui/src/data-services/models/occurrence.tsui/src/data-services/models/pipeline.tsui/src/data-services/models/processing-service.tsui/src/data-services/models/session.tsui/src/data-services/models/species.tsui/src/design-system/components/button/button.module.scssui/src/design-system/components/button/button.tsxui/src/design-system/components/card/card.tsxui/src/design-system/components/checkbox/checkbox.tsxui/src/design-system/components/combo-box/combo-box-simple/combo-box-simple.tsxui/src/design-system/components/dialog/dialog.tsxui/src/design-system/components/icon-button/icon-button.module.scssui/src/design-system/components/icon-button/icon-button.tsxui/src/design-system/components/icon/icon.module.scssui/src/design-system/components/icon/icon.tsxui/src/design-system/components/image-carousel/image-carousel.tsxui/src/design-system/components/input/input.tsxui/src/design-system/components/navigation/navigation-bar-icon.tsxui/src/design-system/components/navigation/navigation-bar.module.scssui/src/design-system/components/navigation/navigation-bar.tsxui/src/design-system/components/pagination-bar/page-button/page-button.tsxui/src/design-system/components/pagination-bar/pagination-bar.tsxui/src/design-system/components/popover/popover.module.scssui/src/design-system/components/popover/popover.tsxui/src/design-system/components/status-info/status-info.tsxui/src/design-system/components/status-info/types.tsui/src/design-system/components/table/basic-table-cell/basic-table-cell.module.scssui/src/design-system/components/table/column-settings/column-settings.tsxui/src/design-system/components/table/date-table-cell/date-table-cell.tsxui/src/design-system/components/table/table-header/table-header.tsxui/src/design-system/components/tabs/tabs.tsxui/src/design-system/components/toggle-group/toggle-group.tsxui/src/design-system/components/wizard/status-bullet/status-bullet.tsxui/src/design-system/components/wizard/wizard.tsxui/src/pages/auth/login.tsxui/src/pages/auth/reset-password.tsxui/src/pages/auth/sign-up.tsxui/src/pages/captures/captures.tsxui/src/pages/captures/upload-images-dialog/select-images-section/select-images-section.tsxui/src/pages/deployment-details/delete-deployment-dialog.tsxui/src/pages/deployment-details/deployment-details-form/deployment-details-form.tsxui/src/pages/deployment-details/deployment-details-form/section-general/section-general.tsxui/src/pages/deployment-details/deployment-details-form/section-location/location-map/location-map.tsxui/src/pages/deployment-details/deployment-details-form/section-location/section-location.tsxui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsxui/src/pages/deployment-details/deployment-details-form/section-source-images/section-source-images.tsxui/src/pages/deployment-details/deployment-details-info.tsxui/src/pages/deployments/deployment-columns.tsxui/src/pages/job-details/job-actions/cancel-job.tsxui/src/pages/job-details/job-actions/queue-job.tsxui/src/pages/job-details/job-actions/retry-job.tsxui/src/pages/job-details/job-details.tsxui/src/pages/jobs/delete-jobs-dialog.tsxui/src/pages/jobs/jobs-columns.tsxui/src/pages/jobs/jobs.tsxui/src/pages/occurrence-details/id-quick-actions/id-button.tsxui/src/pages/occurrence-details/id-quick-actions/id-quick-actions.tsxui/src/pages/occurrences/occurrence-columns.tsxui/src/pages/occurrences/occurrence-gallery.tsxui/src/pages/occurrences/occurrences.tsxui/src/pages/project/algorithms/algorithms-columns.tsxui/src/pages/project/capture-sets/capture-set-columns.tsxui/src/pages/project/entities/details-form/export-details-form.tsxui/src/pages/project/entities/entities-columns.tsxui/src/pages/project/exports/exports-columns.tsxui/src/pages/project/pipelines/pipelines-columns.tsxui/src/pages/project/processing-services/connection-status.tsxui/src/pages/project/processing-services/processing-services-columns.tsxui/src/pages/project/processing-services/status-info/status-info.module.scssui/src/pages/project/processing-services/status-info/status-info.tsxui/src/pages/project/storage/connection-status.tsxui/src/pages/project/storage/status-info/status-info.module.scssui/src/pages/project/storage/status-info/status-info.tsxui/src/pages/project/storage/status-info/types.tsui/src/pages/project/storage/storage-columns.tsxui/src/pages/project/team/team-columns.tsxui/src/pages/session-details/playback/capture-details/capture-job/capture-job-dialog.tsxui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsxui/src/pages/session-details/playback/playback.tsxui/src/pages/sessions/session-columns.tsxui/src/pages/sessions/sessions.tsxui/src/pages/species/species-columns.tsxui/src/pages/species/species.tsxui/src/pages/taxa-lists/taxa-list-columns.tsxui/src/utils/constants.tsui/src/utils/useNavItems.ts
💤 Files with no reviewable changes (17)
- ui/src/pages/project/storage/status-info/types.ts
- ui/src/pages/jobs/jobs.tsx
- ui/src/design-system/components/icon-button/icon-button.module.scss
- ui/src/pages/project/storage/status-info/status-info.module.scss
- ui/src/pages/deployment-details/delete-deployment-dialog.tsx
- ui/src/pages/project/processing-services/status-info/status-info.module.scss
- ui/src/design-system/components/icon/icon.module.scss
- ui/src/components/blueprint-collection/blueprint-collection.tsx
- ui/src/pages/jobs/delete-jobs-dialog.tsx
- ui/src/utils/useNavItems.ts
- ui/src/data-services/models/processing-service.ts
- ui/src/design-system/components/button/button.module.scss
- ui/src/design-system/components/button/button.tsx
- ui/src/pages/project/processing-services/status-info/status-info.tsx
- ui/src/pages/project/storage/status-info/status-info.tsx
- ui/src/design-system/components/icon-button/icon-button.tsx
- ui/src/design-system/components/icon/icon.tsx
ui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/pages/occurrences/occurrence-gallery.tsx (1)
236-236:⚠️ Potential issue | 🟡 MinorHardcoded strings should use translation system.
The strings
'Deselect all'and'Select all'are hardcoded while the rest of the component usestranslate()for i18n. For consistency, consider using translated strings.Suggested fix
- <span>{allSelected ? 'Deselect all' : 'Select all'}</span> + <span>{allSelected ? translate(STRING.DESELECT_ALL) : translate(STRING.SELECT_ALL)}</span>Ensure
STRING.DESELECT_ALLandSTRING.SELECT_ALLare defined in the language constants.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/occurrences/occurrence-gallery.tsx` at line 236, The span currently renders hardcoded labels using allSelected ('Deselect all' / 'Select all'); replace these literals with translated strings using the existing translate() and STRING constants (e.g. STRING.DESELECT_ALL and STRING.SELECT_ALL) so the component uses i18n consistently—update the JSX around the allSelected usage to call translate(STRING.SELECT_ALL) and translate(STRING.DESELECT_ALL) and ensure those keys are added to the language constants.
🧹 Nitpick comments (1)
ui/src/components/filtering/filter-section.tsx (1)
25-33: Consider a more descriptive label for the toggle action.The
STRING.EXPANDlabel is slightly misleading since this button both expands and collapses the section. A neutral label like "Toggle filters" would more accurately describe the action regardless of the current state.That said, the
Collapsible.Triggerlikely setsaria-expandedinternally, so screen readers will still convey the current state. This is a minor improvement if you have a suitable string available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/filtering/filter-section.tsx` around lines 25 - 33, The aria-label for the toggle button currently uses STRING.EXPAND which is misleading for a control that both expands and collapses; update the Button inside Collapsible.Trigger to use a neutral label (e.g., translate(STRING.TOGGLE_FILTERS) or an existing "Toggle filters" string) instead of STRING.EXPAND so the aria-label accurately describes the action while Collapsible.Trigger continues to manage aria-expanded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/src/design-system/components/input/input.tsx`:
- Around line 95-104: The EyeIcon button in the Input component is missing an
accessible name; update the Button (the icon-only control that calls setType and
reads the local type state) to include an appropriate aria-label that reflects
the current visibility state (e.g., when type === 'password' use "Show
password", otherwise "Hide password") so screen readers get a clear, dynamic
name; optionally also add a title attribute matching the aria-label for tooltip
support.
- Around line 137-139: The branch that checks _.isDate(value) should also verify
the Date is valid before calling getFormatedDateTimeString and otherwise return
VALUE_NOT_AVAILABLE; update the condition to check _.isDate(value) &&
!Number.isNaN((value as Date).getTime()) (or a similar validity check) so
getFormatedDateTimeString is only called for real dates, and return
VALUE_NOT_AVAILABLE for invalid Date objects.
---
Outside diff comments:
In `@ui/src/pages/occurrences/occurrence-gallery.tsx`:
- Line 236: The span currently renders hardcoded labels using allSelected
('Deselect all' / 'Select all'); replace these literals with translated strings
using the existing translate() and STRING constants (e.g. STRING.DESELECT_ALL
and STRING.SELECT_ALL) so the component uses i18n consistently—update the JSX
around the allSelected usage to call translate(STRING.SELECT_ALL) and
translate(STRING.DESELECT_ALL) and ensure those keys are added to the language
constants.
---
Nitpick comments:
In `@ui/src/components/filtering/filter-section.tsx`:
- Around line 25-33: The aria-label for the toggle button currently uses
STRING.EXPAND which is misleading for a control that both expands and collapses;
update the Button inside Collapsible.Trigger to use a neutral label (e.g.,
translate(STRING.TOGGLE_FILTERS) or an existing "Toggle filters" string) instead
of STRING.EXPAND so the aria-label accurately describes the action while
Collapsible.Trigger continues to manage aria-expanded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 45f0edc9-04ac-4a13-a741-ec0ab347b993
📒 Files selected for processing (46)
ui/src/components/cookie-dialog/cookie-dialog.module.scssui/src/components/filtering/default-filter-control.tsxui/src/components/filtering/filter-control.tsxui/src/components/filtering/filter-section.tsxui/src/components/taxon-tags/tags-form.tsxui/src/components/terms-of-service-info/terms-of-service-info.tsxui/src/data-services/models/capture-details.tsui/src/data-services/models/capture-set.tsui/src/data-services/models/deployment.tsui/src/data-services/models/session.tsui/src/design-system/components/bulk-action-bar/bulk-action-bar.tsxui/src/design-system/components/image-carousel/image-carousel.tsxui/src/design-system/components/input/input.tsxui/src/design-system/components/page-header/page-header.tsxui/src/design-system/components/pagination-bar/pagination-bar.tsxui/src/pages/auth/sign-up.tsxui/src/pages/captures/upload-images-dialog/select-images-section/select-images-section.tsxui/src/pages/captures/upload-images-dialog/upload-images-dialog.tsxui/src/pages/deployment-details/deployment-details-form/deployment-details-form.tsxui/src/pages/deployment-details/deployment-details-form/section-general/section-general.tsxui/src/pages/deployment-details/deployment-details-form/section-location/location-map/location-map.tsxui/src/pages/deployment-details/deployment-details-form/section-location/section-location.tsxui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsxui/src/pages/deployment-details/deployment-details-form/section-source-images/section-source-images.tsxui/src/pages/job-details/job-actions/cancel-job.tsxui/src/pages/job-details/job-actions/queue-job.tsxui/src/pages/job-details/job-actions/retry-job.tsxui/src/pages/occurrence-details/id-quick-actions/id-button.tsxui/src/pages/occurrence-details/id-quick-actions/id-quick-actions.tsxui/src/pages/occurrence-details/identification-card/human-identification.tsxui/src/pages/occurrences/occurrence-columns.tsxui/src/pages/occurrences/occurrence-gallery.tsxui/src/pages/occurrences/occurrence-navigation.tsxui/src/pages/project-details/default-filters-form.tsxui/src/pages/project-details/delete-project-dialog.tsxui/src/pages/project/entities/delete-entity-dialog.tsxui/src/pages/project/entities/details-form/capture-set-details-form.tsxui/src/pages/project/entities/entities-columns.tsxui/src/pages/project/entities/entity-details-dialog.tsxui/src/pages/project/pipelines/pipelines-columns.tsxui/src/pages/project/team/leave-team-dialog.tsxui/src/pages/project/team/remove-member-dialog.tsxui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsxui/src/pages/session-details/playback/frame/frame.tsxui/src/utils/constants.tsui/src/utils/language.ts
✅ Files skipped from review due to trivial changes (17)
- ui/src/pages/occurrence-details/identification-card/human-identification.tsx
- ui/src/components/taxon-tags/tags-form.tsx
- ui/src/pages/project/entities/details-form/capture-set-details-form.tsx
- ui/src/pages/captures/upload-images-dialog/upload-images-dialog.tsx
- ui/src/pages/project/entities/delete-entity-dialog.tsx
- ui/src/pages/occurrences/occurrence-navigation.tsx
- ui/src/components/cookie-dialog/cookie-dialog.module.scss
- ui/src/pages/project-details/delete-project-dialog.tsx
- ui/src/pages/deployment-details/deployment-details-form/section-source-images/section-source-images.tsx
- ui/src/components/filtering/filter-control.tsx
- ui/src/utils/language.ts
- ui/src/design-system/components/bulk-action-bar/bulk-action-bar.tsx
- ui/src/design-system/components/pagination-bar/pagination-bar.tsx
- ui/src/pages/deployment-details/deployment-details-form/deployment-details-form.tsx
- ui/src/pages/captures/upload-images-dialog/select-images-section/select-images-section.tsx
- ui/src/pages/project/entities/entity-details-dialog.tsx
- ui/src/pages/project/entities/entities-columns.tsx
🚧 Files skipped from review as they are similar to previous changes (17)
- ui/src/pages/deployment-details/deployment-details-form/section-general/section-general.tsx
- ui/src/data-services/models/capture-set.ts
- ui/src/data-services/models/session.ts
- ui/src/utils/constants.ts
- ui/src/pages/occurrence-details/id-quick-actions/id-quick-actions.tsx
- ui/src/data-services/models/deployment.ts
- ui/src/components/terms-of-service-info/terms-of-service-info.tsx
- ui/src/pages/project/pipelines/pipelines-columns.tsx
- ui/src/pages/deployment-details/deployment-details-form/section-location/location-map/location-map.tsx
- ui/src/pages/job-details/job-actions/queue-job.tsx
- ui/src/pages/job-details/job-actions/cancel-job.tsx
- ui/src/pages/job-details/job-actions/retry-job.tsx
- ui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsx
- ui/src/pages/auth/sign-up.tsx
- ui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsx
- ui/src/data-services/models/capture-details.ts
- ui/src/components/filtering/default-filter-control.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsx (1)
90-112: Optional: deduplicate repeated button style props.Both nav buttons repeat the same
classNameandsize. Extracting a shared constant reduces drift risk.♻️ Proposed small cleanup
+ const navButtonClassName = '!bg-neutral-700 text-neutral-200' + return ( <div className={styles.wrapper}> <Button aria-label={translate(STRING.PREVIOUS)} - className="!bg-neutral-700 text-neutral-200" + className={navButtonClassName} disabled={!activeCapture?.prevCaptureId} onClick={goToPrev} size="icon" > <ChevronLeftIcon className="w-4 h-4" /> </Button> @@ <Button aria-label={translate(STRING.NEXT)} - className="!bg-neutral-700 text-neutral-200" + className={navButtonClassName} disabled={!activeCapture?.nextCaptureId} onClick={goToNext} size="icon" > <ChevronRightIcon className="w-4 h-4" /> </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsx` around lines 90 - 112, The two navigation Button instances (the ones using ChevronLeftIcon/ChevronRightIcon and handlers goToPrev/goToNext) duplicate the same className and size props; introduce a shared constant (e.g., NAV_BUTTON_PROPS) near this component and spread it into both Buttons to remove duplication while keeping unique props like aria-label, disabled, and onClick inline; update the left/right Button usages to use the shared props and ensure nothing else (activeCapture?.prevCaptureId/nextCaptureId, currentIndex/totalCaptures) is changed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/src/data-services/models/export.ts`:
- Line 50: The current assignment "const key = _key.replace('collection',
'capture set')" uses a blind substring replace that can mutate unrelated tokens
and introduces a space; change this to do a targeted replacement on the variable
_key (e.g., match whole-word "collection" or specific tokens like "collection_"
or "-collection") and replace with a snake_case token like "capture_set", then
apply the existing snake-case normalization logic so no spaces are injected;
update the code around the variables _key and key in models/export.ts
accordingly and remove/adjust the temporary TODO comment.
---
Nitpick comments:
In
`@ui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsx`:
- Around line 90-112: The two navigation Button instances (the ones using
ChevronLeftIcon/ChevronRightIcon and handlers goToPrev/goToNext) duplicate the
same className and size props; introduce a shared constant (e.g.,
NAV_BUTTON_PROPS) near this component and spread it into both Buttons to remove
duplication while keeping unique props like aria-label, disabled, and onClick
inline; update the left/right Button usages to use the shared props and ensure
nothing else (activeCapture?.prevCaptureId/nextCaptureId,
currentIndex/totalCaptures) is changed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8bd73aee-4bcd-4807-bcf1-6e0983da8c2d
📒 Files selected for processing (5)
ui/src/data-services/models/export.tsui/src/pages/captures/upload-images-dialog/upload-images-dialog.tsxui/src/pages/project/capture-sets/capture-sets.tsxui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsxui/src/pages/session-details/playback/playback.tsx
✅ Files skipped from review due to trivial changes (1)
- ui/src/pages/project/capture-sets/capture-sets.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- ui/src/pages/captures/upload-images-dialog/upload-images-dialog.tsx
- ui/src/pages/session-details/playback/playback.tsx
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
1 participant
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 of changes
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements
Other Changes