Merged
Conversation
✅ Deploy Preview for antenna-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for antenna-ssec ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughStandardizes many table column factories to object-parameter signatures, adds InfoTooltip and Toolbar design-system components, introduces sticky column support and per-field tooltips, adjusts several data models (detection scoreLabel, Species.lastSeen), and applies widespread UI/spacing/i18n updates across pages and components. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
annavik
force-pushed
the
chore/ui-cleanup
branch
from
fdf1f2a to
4b8e07e
Compare
annavik
force-pushed
the
chore/ui-cleanup
branch
from
f4b9d21 to
ef7ccec
Compare
![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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/src/pages/taxa-list-details/add-taxa-list-taxon/add-taxa-list-taxon-popover.tsx (1)
30-39:⚠️ Potential issue | 🔴 CriticalNon-compact button is missing
Popover.Trigger, so the popover won't open.The non-compact branch renders a
<Button>without wrapping it in<Popover.Trigger>. Clicking this button will not open the popover—it's effectively broken.Proposed fix
</BasicTooltip> ) : ( + <Popover.Trigger asChild> <Button size="small" variant="outline"> <PlusIcon className="w-4 h-4" /> <span> {translate(STRING.ENTITY_ADD, { type: translate(STRING.ENTITY_TYPE_TAXON), })} </span> </Button> + </Popover.Trigger> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/taxa-list-details/add-taxa-list-taxon/add-taxa-list-taxon-popover.tsx` around lines 30 - 39, In the non-compact rendering branch of the AddTaxaListTaxonPopover component (file add-taxa-list-taxon-popover.tsx) the Button with PlusIcon is rendered directly and therefore won't open the popover; wrap that Button inside a Popover.Trigger so the button activates the popover (use Popover.Trigger around the Button in the same way the compact branch does), ensuring the existing Button, PlusIcon, and translated label remain unchanged.ui/src/pages/project/processing-services/processing-services-actions.tsx (1)
18-24:⚠️ Potential issue | 🟡 MinorKeep the error tooltip copy translated.
This control is icon-only now, so the tooltip is the only visible explanation. The error branch is still hardcoded English, which will bypass locale switching.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/project/processing-services/processing-services-actions.tsx` around lines 18 - 24, The tooltip content currently uses a hard-coded English string when error is truthy; update the BasicTooltip content to always call translate(...) instead of raw text so the error message is localized (e.g., replace the error branch with translate(STRING.REGISTER_PIPELINES_ERROR) or another appropriate key), ensuring you add the new translation key to the i18n resource if needed and keep the existing non-error branch using translate(STRING.REGISTER_PIPELINES).
🧹 Nitpick comments (8)
ui/src/design-system/components/input/input.tsx (1)
130-136: Consider extracting duplicated tooltip type.The tooltip type definition is duplicated between
InputValue(lines 130-136) andInputContent(lines 180-186). Consider extracting to a shared type for consistency and maintainability.♻️ Optional: Extract shared tooltip type
// Add near the top of the file type TooltipConfig = { text: string link?: { text: string to: string } } // Then use in both components tooltip?: TooltipConfigAlso applies to: 180-186
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/design-system/components/input/input.tsx` around lines 130 - 136, Extract the duplicated tooltip object into a shared type (e.g., TooltipConfig) and use it in both InputValue and InputContent; specifically, add a type declaration near the top of the file that captures { text: string; link?: { text: string; to: string } } and replace the inline tooltip?: {...} occurrences in the InputValue and InputContent props/definitions with tooltip?: TooltipConfig to ensure consistency and maintainability.ui/src/pages/project/pipelines/default-pipeline-badge.tsx (1)
14-20: Hardcoded tooltip text inconsistent with i18n pattern.The tooltip text on line 15 is hardcoded in English while the link text on line 17 uses
translate(STRING.CONFIGURE). Consider extracting the tooltip text to aSTRINGconstant for consistency with the i18n approach used elsewhere in this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/project/pipelines/default-pipeline-badge.tsx` around lines 14 - 20, The tooltip text passed to the Info component is hardcoded; extract it into the i18n constants and use translate like the link text. Add a new STRING key (e.g., DEFAULT_PIPELINE_TOOLTIP) in the same STRING collection and replace the literal "This is the default pipeline used for processing images in this project." with translate(STRING.DEFAULT_PIPELINE_TOOLTIP) in the Info call (located in the DefaultPipelineBadge component where Info, translate, STRING, APP_ROUTES and projectId are used).ui/src/pages/deployments/deployment-columns.tsx (1)
227-235: Consider conditionally renderingToolbaronly when actions are available.The
Toolbarcomponent renders styled container divs even when empty (whenitem.canDeleteis false). This results in unnecessary DOM elements with padding, background color, rounded corners, and hover styling applied despite having no actionable content. To avoid this, either:
- Conditionally render
<Toolbar>only when at least one action is available- Or refactor the
Toolbarcomponent to render nothing whenchildrenis empty🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/deployments/deployment-columns.tsx` around lines 227 - 235, The Toolbar wrapper is rendered even when empty causing unnecessary styled DOM; update the render in deployment-columns.tsx so Toolbar is only output when there's at least one action (e.g., wrap <Toolbar>...</Toolbar> with a condition like item.canDelete) or alternatively update the Toolbar component itself to return null when children is empty; locate the usage around the DeleteEntityDialog and the Toolbar symbol to apply the conditional rendering or the early-return fix.ui/src/pages/project/entities/entities-columns.tsx (1)
56-72: Action order: Delete appears before Update.The actions render Delete first, then Update. Typically, destructive actions (Delete) are placed last to reduce accidental clicks and follow common UX patterns. If this ordering is intentional (e.g., to match a design spec), please disregard.
🎨 Consider reordering actions
<Toolbar> - {item.canDelete && ( - <DeleteEntityDialog - collection={collection} - id={item.id} - type={type} - /> - )} {item.canUpdate && ( <UpdateEntityDialog collection={collection} entity={item} type={type} isCompact /> )} + {item.canDelete && ( + <DeleteEntityDialog + collection={collection} + id={item.id} + type={type} + /> + )} </Toolbar>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/project/entities/entities-columns.tsx` around lines 56 - 72, The action order in the Toolbar currently renders DeleteEntityDialog before UpdateEntityDialog; move DeleteEntityDialog to render after UpdateEntityDialog so destructive actions appear last. Locate the Toolbar JSX where DeleteEntityDialog and UpdateEntityDialog are used (props: collection, id/entity, type, isCompact) and swap their positions so UpdateEntityDialog is first and DeleteEntityDialog is second, preserving all existing props and conditional checks (item.canUpdate, item.canDelete).ui/src/pages/algorithm-details/algorithm-details-dialog.tsx (1)
76-85: Stray blank line between JSX elements.Line 80 has an extra blank line inside the
FormRowbetweenInputValueelements. This may be unintentional and could affect formatting consistency.🧹 Proposed fix
<InputValue label={translate(STRING.FIELD_LABEL_TASK_TYPE)} value={algorithm.taskType} /> - <InputValue label={translate(STRING.FIELD_LABEL_CATEGORY_COUNT)} value={algorithm.categoryCount} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/algorithm-details/algorithm-details-dialog.tsx` around lines 76 - 85, Remove the stray blank line inside the FormRow between the two InputValue JSX elements; in the AlgorithmDetailsDialog component (the JSX that renders <FormRow>), place the <InputValue label={translate(STRING.FIELD_LABEL_TASK_TYPE)} value={algorithm.taskType} /> and the <InputValue label={translate(STRING.FIELD_LABEL_CATEGORY_COUNT)} value={algorithm.categoryCount} /> directly adjacent (no extra blank line) to restore consistent JSX formatting.ui/src/pages/project/processing-services/processing-services-columns.tsx (1)
87-106:UpdateEntityDialogmay be missingisCompactprop.In
entities-columns.tsx,UpdateEntityDialogreceivesisCompactwhen rendered inside aToolbar. Here it's omitted, which may cause inconsistent button styling.🎨 Consider adding isCompact for consistency
<UpdateEntityDialog collection={API_ROUTES.PROCESSING_SERVICES} entity={item} type="service" + isCompact />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/project/processing-services/processing-services-columns.tsx` around lines 87 - 106, The UpdateEntityDialog rendered inside the Toolbar for processing services is missing the isCompact prop which causes inconsistent button styling; update the JSX where UpdateEntityDialog is used (next to PopulateProcessingService and DeleteEntityDialog within the Toolbar when item.canUpdate is true) to pass isCompact (e.g., isCompact={true}) so its appearance matches the other Toolbar usage in entities-columns.tsx while keeping collection={API_ROUTES.PROCESSING_SERVICES}, entity={item}, and type="service" intact.ui/src/pages/occurrences/occurrences-actions.tsx (1)
62-80: Reuseoccurrence.userAgreed()when buildingagreeParams.
allAgreedand the payload filter are using different "already agreed" checks right now. IfOccurrence.userAgreed()ever diverges fromdeterminationVerifiedBy?.id, the toolbar can still submit occurrences it already considers agreed. Reusing the model helper here also fixes the missinguserInfo?.idmemo dependency.Possible cleanup
const agreeParams: IdentificationFieldValues[] = useMemo( () => occurrences - .filter((occurrences) => { - const agreed = userInfo?.id - ? userInfo.id === occurrences.determinationVerifiedBy?.id - : false - - return !agreed - }) + .filter((occurrence) => + userInfo?.id ? !occurrence.userAgreed(userInfo.id) : true + ) .map((occurrence) => ({ agreeWith: { identificationId: occurrence.determinationIdentificationId, predictionId: occurrence.determinationPredictionId, }, occurrenceId: occurrence.id, taxonId: occurrence.determinationTaxon.id, })), - [occurrences] + [occurrences, userInfo?.id] )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/occurrences/occurrences-actions.tsx` around lines 62 - 80, The agreeParams builder should reuse the model helper to determine if a user already agreed instead of checking determinationVerifiedBy?.id directly; update the filter in agreeParams to call occurrence.userAgreed(userInfo?.id) (or the correct Occurrence.userAgreed signature) and add userInfo?.id to the useMemo dependency array so the memo updates when the current user changes; keep the rest of the mapping (agreeWith, occurrenceId, taxonId) unchanged.ui/src/utils/useFilters.ts (1)
23-160: Consider extracting a small tooltip-link helper to reduce duplication.The repeated
{ text, link: { text, to } }blocks are consistent but verbose; a helper would make this list easier to maintain.♻️ Suggested refactor pattern
+const makeTooltipLink = (text: string, linkText: string, to: string) => ({ + text, + link: { text: linkText, to }, +}) + export const AVAILABLE_FILTERS = (projectId: string): FilterConfig[] => [ { label: 'Include algorithm', field: 'algorithm', - tooltip: { - text: translate(STRING.TOOLTIP_ALGORITHM), - link: { - text: translate(STRING.NAV_ITEM_ALGORITHMS), - to: APP_ROUTES.ALGORITHMS({ projectId }), - }, - }, + tooltip: makeTooltipLink( + translate(STRING.TOOLTIP_ALGORITHM), + translate(STRING.NAV_ITEM_ALGORITHMS), + APP_ROUTES.ALGORITHMS({ projectId }) + ), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/utils/useFilters.ts` around lines 23 - 160, The AVAILABLE_FILTERS array repeats identical tooltip-link objects; extract a small helper (e.g., buildTooltip or makeTooltip) that accepts tooltipTextKey and routeFactory (or projectId) and returns { text: translate(...), link: { text: translate(...), to: ... } } and replace each inline tooltip with calls to that helper; update usages inside AVAILABLE_FILTERS (references: tooltip objects in entries for 'algorithm', 'collection', 'source_image_collection', 'collections', 'deployment', capture/date entries, 'event', 'pipeline', 'not_algorithm' and translate/APP_ROUTES) so the list is shorter and DRY.
🤖 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/components/blueprint-collection/blueprint-collection.tsx`:
- Around line 74-80: The "View in session" Link currently has no unique
accessible name; update the Link to include an explicit accessible label by
adding an aria-label (or a visually hidden suffix) that appends the blueprint
identifier (use item.label || item.timeLabel) to the translated
STRING.VIEW_IN_SESSION so each Link announced is unique; modify the Link element
that uses buttonVariants, to set
aria-label={`{translate(STRING.VIEW_IN_SESSION)} ${item.label ||
item.timeLabel}`} (or add an adjacent visually hidden span with that text) while
keeping the visible content and ChevronRightIcon unchanged.
In `@ui/src/components/filtering/default-filter-control.tsx`:
- Around line 68-70: The new hardcoded UI copy in default-filter-control.tsx
(the paragraph and other text nodes at the <p className="body-small italic
text-muted-foreground"> elements and the sibling text nodes around lines showing
the default filter messaging) must be moved to i18n keys; replace the raw
English strings with translation lookups (e.g., using the project's translation
hook/function like useTranslation().t or t('namespace.key')) and add
corresponding keys and English values to the translation resource
file/namespace. Update the component (DefaultFilterControl or the relevant
exported function) to import and use the translation helper, replace each
hardcoded sentence with a unique key (e.g., 'filter.defaultDescription',
'filter.appliedLabel', etc.), and ensure tests/build pick up the new keys.
- Around line 65-66: The Tooltip usage around the interactive "Configure" link
in default-filter-control.tsx (specifically the Tooltip.Content instances around
the configure UI at the blocks referencing lines 65-66 and 92-103) is
inaccessible because Radix Tooltip closes on keyboard events and prevents
focusing interactive elements; replace the Tooltip wrapper with a Radix Popover
(Popover.Root, Popover.Trigger, Popover.Content) so focus is trapped and
keyboard navigation works, move the existing content into Popover.Content,
ensure the Configure link becomes a focusable element inside Popover.Content,
and add appropriate aria attributes/close handling using Popover APIs to
preserve current styling and behavior.
In `@ui/src/components/filtering/filters/verification-status-filter.tsx`:
- Around line 6-9: The module-level OPTIONS array uses translate(...) at import
time and will not update on locale changes; instead, remove the top-level
OPTIONS constant and construct the options inside the component render (e.g.,
inside the VerificationStatusFilter function or its render body) by calling
translate(STRING.VERIFIED) and translate(STRING.NOT_VERIFIED) there so labels
are recomputed on locale change; keep the same option shape ({ value:
true/false, label: ... }) and update any references to OPTIONS to use the newly
built in-render array.
In
`@ui/src/data-services/hooks/processing-services/useProcessingServiceDetails.ts`:
- Around line 23-24: The queryKey used in useProcessingServiceDetails (currently
[API_ROUTES.PROCESSING_SERVICES, id]) must include projectId to avoid cache
collisions across projects; update the queryKey to include projectId (e.g.,
[API_ROUTES.PROCESSING_SERVICES, id, projectId]) so React Query differentiates
responses for different project contexts while leaving the URL
(`${API_URL}/${API_ROUTES.PROCESSING_SERVICES}/${id}/?project_id=${projectId}`)
unchanged.
In `@ui/src/design-system/components/table/table/table.module.scss`:
- Around line 95-111: The sticky action cells are only made visible on mouse
hover (tr:hover) which hides them from keyboard users; update the stylesheet so
the visibility rules apply also when a row or cell has keyboard focus by adding
:focus-within parity (e.g., use tr:hover, tr:focus-within to set
background-color and reveal td.sticky, and ensure td.sticky also becomes opaque
on :focus-within) so selectors like tr:hover and td.sticky are mirrored with
:focus-within to expose controls for keyboard users.
In `@ui/src/design-system/components/table/table/table.tsx`:
- Around line 156-169: The current split into non-sticky and sticky groups calls
.filter() before .map(), which resets the columnIndex passed to
column.renderCell; update the rendering so the original index from the full
columns array is preserved when calling renderCell. Concretely, iterate the
original columns with columns.map((column, columnIndex) => ...) and inside that
map render non-sticky cells in the non-sticky section (or collect them) and
render sticky cells in the sticky section while passing the same columnIndex to
column.renderCell; ensure any keys still use column.id and keep the sticky
className applied for sticky columns.
In `@ui/src/pages/occurrences/occurrences-actions.tsx`:
- Around line 95-109: The confirm Button remains clickable during the
createIdentifications mutation; prevent duplicate requests by disabling the
Button while isLoading is true. Update the Button component (the one that calls
createIdentifications with agreeParams) to include a disabled={isLoading} prop
(and optionally aria-busy or data-loading attribute) and ensure the onClick
handler is no-op when isLoading to avoid double-invocation.
- Around line 27-47: The toolbar currently authorizes bulk actions based only on
occurrences[0] via canUpdate; change this so authorization is based on every
selected occurrence or only passes allowed items: either (A) compute canUpdate =
occurrences.every(o => o.userPermissions.includes(UserPermission.Update)) and
return null if false, or (B) compute allowedOccurrences = occurrences.filter(o
=> o.userPermissions.includes(UserPermission.Update)), hide/disable the toolbar
when allowedOccurrences.length === 0, and pass allowedOccurrences.map(o => o.id)
and allowedOccurrences.map(o => o.determinationTaxon) into Agree,
SuggestIdPopover, and IdQuickActions instead of using occurrences or
occurrences[0]; update references to canUpdate, occurrences[0], Agree,
SuggestIdPopover, and IdQuickActions accordingly.
In `@ui/src/pages/project/capture-sets/populate-capture-set.tsx`:
- Around line 33-39: The loading spinner (Loader2) in the JSX conditional block
is using an extra class "ml-2" which offsets it compared to AlertCircleIcon and
RefreshCcwIcon; remove the "ml-2" from the Loader2 class list so all three icons
(Loader2, AlertCircleIcon, RefreshCcwIcon) share the same sizing/spacing ("w-4
h-4" etc.) in the render block inside populate-capture-set.tsx to keep the
icon-only button visually centered.
In `@ui/src/pages/project/pipelines/pipelines.tsx`:
- Around line 18-22: The pipelines table's column settings are initialized with
only id, name, and description which causes any other column like "default" to
be treated as falsy and hidden; update the initialization passed to
useColumnSettings in pipelines.tsx to include default: true (i.e., include the
"default" key in the initial settings) or alternatively change the hide logic
that reads columnSettings (where columns are hidden when a key is not truthy) to
treat undefined/missing keys as visible by default; reference useColumnSettings,
columnSettings, setColumnSettings and the pipelines table render/hide logic when
making the change.
In `@ui/src/pages/project/processing-services/processing-services-actions.tsx`:
- Around line 34-39: The loading spinner has an extra left margin that shifts it
off-center—remove the "ml-2" class from the Loader2 element in
processing-services-actions.tsx so its className becomes "w-4 h-4 animate-spin"
(keep the existing conditional rendering that uses error, isLoading and the
icons RefreshCcwIcon/AlertCircleIcon intact).
In `@ui/src/pages/session-details/playback/frame/frame.tsx`:
- Around line 256-259: The DeterminationScore component call is missing the
verified prop so the label and internal verified state can go out of sync;
update the JSX where DeterminationScore is rendered (the instance passing
score={detection.score} and scoreLabel={detection.scoreLabel}) to also pass
verified={detection.verified} so the component receives the detection's verified
flag and can render the correct score state and label.
In `@ui/src/pages/taxa-list-details/taxa-list-details-columns.tsx`:
- Around line 17-23: The RemoveTaxaListTaxonDialog is rendered unconditionally
in the columns factory allowing unauthorized users to open the removal flow; add
an authorization check so the remove trigger is hidden/disabled for users
without update rights: either pass a canUpdate boolean into
RemoveTaxaListTaxonDialog from the columns function (which already accepts
projectId and taxaListId) and use it to disable/hide the trigger button, or have
RemoveTaxaListTaxonDialog itself query the current user's permissions and
early-return/disable the trigger when update is not allowed; update the
columns(...) implementation and the RemoveTaxaListTaxonDialog props/behavior
accordingly to prevent unauthorized interaction.
In `@ui/src/pages/taxa-list-details/taxa-list-details.tsx`:
- Around line 51-54: The table's delete action is now exposed because
`canUpdate` was removed from the `columns(...)` config; either restore
permission gating when building columns or gate the dialog itself. Update the
parent in taxa-list-details to pass `canUpdate` into the `columns({ projectId,
taxaListId, canUpdate })` call and use that flag to avoid rendering
`RemoveTaxaListTaxonDialog` (similar to how `AddTaxaListTaxonPopover` is
guarded), or add an internal check in `RemoveTaxaListTaxonDialog` to
early-return/null when `canUpdate` is false; ensure the same fix is applied at
the other referenced actions (lines 69–72) so delete actions are hidden for
users without update permission.
---
Outside diff comments:
In `@ui/src/pages/project/processing-services/processing-services-actions.tsx`:
- Around line 18-24: The tooltip content currently uses a hard-coded English
string when error is truthy; update the BasicTooltip content to always call
translate(...) instead of raw text so the error message is localized (e.g.,
replace the error branch with translate(STRING.REGISTER_PIPELINES_ERROR) or
another appropriate key), ensuring you add the new translation key to the i18n
resource if needed and keep the existing non-error branch using
translate(STRING.REGISTER_PIPELINES).
In
`@ui/src/pages/taxa-list-details/add-taxa-list-taxon/add-taxa-list-taxon-popover.tsx`:
- Around line 30-39: In the non-compact rendering branch of the
AddTaxaListTaxonPopover component (file add-taxa-list-taxon-popover.tsx) the
Button with PlusIcon is rendered directly and therefore won't open the popover;
wrap that Button inside a Popover.Trigger so the button activates the popover
(use Popover.Trigger around the Button in the same way the compact branch does),
ensuring the existing Button, PlusIcon, and translated label remain unchanged.
---
Nitpick comments:
In `@ui/src/design-system/components/input/input.tsx`:
- Around line 130-136: Extract the duplicated tooltip object into a shared type
(e.g., TooltipConfig) and use it in both InputValue and InputContent;
specifically, add a type declaration near the top of the file that captures {
text: string; link?: { text: string; to: string } } and replace the inline
tooltip?: {...} occurrences in the InputValue and InputContent props/definitions
with tooltip?: TooltipConfig to ensure consistency and maintainability.
In `@ui/src/pages/algorithm-details/algorithm-details-dialog.tsx`:
- Around line 76-85: Remove the stray blank line inside the FormRow between the
two InputValue JSX elements; in the AlgorithmDetailsDialog component (the JSX
that renders <FormRow>), place the <InputValue
label={translate(STRING.FIELD_LABEL_TASK_TYPE)} value={algorithm.taskType} />
and the <InputValue label={translate(STRING.FIELD_LABEL_CATEGORY_COUNT)}
value={algorithm.categoryCount} /> directly adjacent (no extra blank line) to
restore consistent JSX formatting.
In `@ui/src/pages/deployments/deployment-columns.tsx`:
- Around line 227-235: The Toolbar wrapper is rendered even when empty causing
unnecessary styled DOM; update the render in deployment-columns.tsx so Toolbar
is only output when there's at least one action (e.g., wrap
<Toolbar>...</Toolbar> with a condition like item.canDelete) or alternatively
update the Toolbar component itself to return null when children is empty;
locate the usage around the DeleteEntityDialog and the Toolbar symbol to apply
the conditional rendering or the early-return fix.
In `@ui/src/pages/occurrences/occurrences-actions.tsx`:
- Around line 62-80: The agreeParams builder should reuse the model helper to
determine if a user already agreed instead of checking
determinationVerifiedBy?.id directly; update the filter in agreeParams to call
occurrence.userAgreed(userInfo?.id) (or the correct Occurrence.userAgreed
signature) and add userInfo?.id to the useMemo dependency array so the memo
updates when the current user changes; keep the rest of the mapping (agreeWith,
occurrenceId, taxonId) unchanged.
In `@ui/src/pages/project/entities/entities-columns.tsx`:
- Around line 56-72: The action order in the Toolbar currently renders
DeleteEntityDialog before UpdateEntityDialog; move DeleteEntityDialog to render
after UpdateEntityDialog so destructive actions appear last. Locate the Toolbar
JSX where DeleteEntityDialog and UpdateEntityDialog are used (props: collection,
id/entity, type, isCompact) and swap their positions so UpdateEntityDialog is
first and DeleteEntityDialog is second, preserving all existing props and
conditional checks (item.canUpdate, item.canDelete).
In `@ui/src/pages/project/pipelines/default-pipeline-badge.tsx`:
- Around line 14-20: The tooltip text passed to the Info component is hardcoded;
extract it into the i18n constants and use translate like the link text. Add a
new STRING key (e.g., DEFAULT_PIPELINE_TOOLTIP) in the same STRING collection
and replace the literal "This is the default pipeline used for processing images
in this project." with translate(STRING.DEFAULT_PIPELINE_TOOLTIP) in the Info
call (located in the DefaultPipelineBadge component where Info, translate,
STRING, APP_ROUTES and projectId are used).
In `@ui/src/pages/project/processing-services/processing-services-columns.tsx`:
- Around line 87-106: The UpdateEntityDialog rendered inside the Toolbar for
processing services is missing the isCompact prop which causes inconsistent
button styling; update the JSX where UpdateEntityDialog is used (next to
PopulateProcessingService and DeleteEntityDialog within the Toolbar when
item.canUpdate is true) to pass isCompact (e.g., isCompact={true}) so its
appearance matches the other Toolbar usage in entities-columns.tsx while keeping
collection={API_ROUTES.PROCESSING_SERVICES}, entity={item}, and type="service"
intact.
In `@ui/src/utils/useFilters.ts`:
- Around line 23-160: The AVAILABLE_FILTERS array repeats identical tooltip-link
objects; extract a small helper (e.g., buildTooltip or makeTooltip) that accepts
tooltipTextKey and routeFactory (or projectId) and returns { text:
translate(...), link: { text: translate(...), to: ... } } and replace each
inline tooltip with calls to that helper; update usages inside AVAILABLE_FILTERS
(references: tooltip objects in entries for 'algorithm', 'collection',
'source_image_collection', 'collections', 'deployment', capture/date entries,
'event', 'pipeline', 'not_algorithm' and translate/APP_ROUTES) so the list is
shorter and DRY.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 116ea290-72c9-42f5-968f-c99fd8c4c8bb
⛔ Files ignored due to path filters (1)
ui/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (87)
ui/package.jsonui/src/components/blueprint-collection/blueprint-collection.module.scssui/src/components/blueprint-collection/blueprint-collection.tsxui/src/components/determination-score.tsxui/src/components/filtering/default-filter-control.tsxui/src/components/filtering/filter-control.tsxui/src/components/filtering/filters/image-filter.tsxui/src/components/filtering/filters/session-filter.tsxui/src/components/filtering/filters/verification-status-filter.tsxui/src/components/form/delete-form/delete-form.tsxui/src/data-services/hooks/processing-services/useProcessingServiceDetails.tsui/src/data-services/models/capture.tsui/src/data-services/models/species.tsui/src/design-system/components/info-tooltip.tsxui/src/design-system/components/input/input.module.scssui/src/design-system/components/input/input.tsxui/src/design-system/components/navigation/navigation-bar-icon.tsxui/src/design-system/components/table/column-settings/column-settings.tsxui/src/design-system/components/table/table-header/table-header.module.scssui/src/design-system/components/table/table/table.module.scssui/src/design-system/components/table/table/table.tsxui/src/design-system/components/table/types.tsui/src/design-system/components/toolbar.tsxui/src/design-system/components/tooltip/basic-tooltip.tsxui/src/pages/algorithm-details/algorithm-details-dialog.tsxui/src/pages/captures/capture-columns.tsxui/src/pages/captures/captures.tsxui/src/pages/captures/upload-images-dialog/upload-images-dialog.tsxui/src/pages/deployment-details/deployment-details-form/section-general/section-general.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/deployments/deployments.tsxui/src/pages/job-details/job-actions/cancel-job.tsxui/src/pages/job-details/job-actions/queue-job.tsxui/src/pages/job-details/job-details-form/job-details-form.tsxui/src/pages/jobs/jobs-columns.tsxui/src/pages/jobs/jobs.tsxui/src/pages/occurrences/occurrence-actions.tsxui/src/pages/occurrences/occurrence-columns.tsxui/src/pages/occurrences/occurrence-gallery.tsxui/src/pages/occurrences/occurrences-actions.tsxui/src/pages/occurrences/occurrences.tsxui/src/pages/pipeline-details/pipeline-algorithms.tsxui/src/pages/pipeline-details/pipeline-details-dialog.tsxui/src/pages/processing-service-details/processing-service-details-dialog.tsxui/src/pages/processing-service-details/processing-service-pipelines.tsxui/src/pages/project/algorithms/algorithms-columns.tsxui/src/pages/project/algorithms/algorithms.tsxui/src/pages/project/capture-sets/capture-set-columns.tsxui/src/pages/project/capture-sets/capture-sets.tsxui/src/pages/project/capture-sets/populate-capture-set.tsxui/src/pages/project/entities/details-form/export-details-form.tsxui/src/pages/project/entities/entities-columns.tsxui/src/pages/project/entities/entities.tsxui/src/pages/project/entities/styles.module.scssui/src/pages/project/exports/exports-columns.tsxui/src/pages/project/exports/exports.tsxui/src/pages/project/pipelines/default-pipeline-badge.tsxui/src/pages/project/pipelines/pipelines-columns.tsxui/src/pages/project/pipelines/pipelines.tsxui/src/pages/project/processing-services/processing-services-actions.tsxui/src/pages/project/processing-services/processing-services-columns.tsxui/src/pages/project/processing-services/processing-services.tsxui/src/pages/project/sidebar/sidebar.tsxui/src/pages/project/storage/storage-columns.tsxui/src/pages/project/storage/storage.tsxui/src/pages/project/team/leave-team-dialog.tsxui/src/pages/project/team/manage-access-dialog.tsxui/src/pages/project/team/team-columns.tsxui/src/pages/project/team/team.tsxui/src/pages/session-details/playback/capture-details/capture-job/capture-job-dialog.tsxui/src/pages/session-details/playback/frame/frame.tsxui/src/pages/session-details/playback/playback.tsxui/src/pages/sessions/session-columns.tsxui/src/pages/sessions/sessions.tsxui/src/pages/species-details/species-details.tsxui/src/pages/species/species-columns.tsxui/src/pages/taxa-list-details/add-taxa-list-taxon/add-taxa-list-taxon-popover.tsxui/src/pages/taxa-list-details/remove-taxa-list-taxon/remove-taxa-list-taxon-dialog.tsxui/src/pages/taxa-list-details/taxa-list-details-columns.tsxui/src/pages/taxa-list-details/taxa-list-details.tsxui/src/pages/taxa-lists/taxa-list-columns.tsxui/src/pages/taxa-lists/taxa-lists.tsxui/src/utils/language.tsui/src/utils/useFilters.tsui/tailwind.config.js
💤 Files with no reviewable changes (3)
- ui/src/design-system/components/table/table-header/table-header.module.scss
- ui/src/design-system/components/input/input.module.scss
- ui/src/pages/project/entities/styles.module.scss
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
ui/src/design-system/components/table/table-header/table-header.tsx (1)
7-14: Unusedstickyprop in interface.The
sticky?: booleanprop at line 12 is defined inTableHeaderPropsbut never used. BothBasicTableHeader(line 48) andSortableTableHeader(line 93) readcolumn.stickydirectly instead. Either remove the unused prop or use it consistently.♻️ Suggested fix: remove unused prop
interface TableHeaderProps<T> { column: TableColumn<T> onSortClick: () => void sortable?: boolean sortSettings?: TableSortSettings - sticky?: boolean visuallyHidden?: boolean }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/design-system/components/table/table-header/table-header.tsx` around lines 7 - 14, The TableHeaderProps interface declares an unused sticky?: boolean prop; remove this property from TableHeaderProps so the code consistently uses column.sticky (used by BasicTableHeader and SortableTableHeader) and update any refs to TableHeaderProps if necessary to rely on column.sticky instead of a separate prop.
🤖 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/pages/algorithm-details/algorithm-details-dialog.tsx`:
- Around line 100-125: The anchor links render unvalidated algorithm.uri and
algorithm.categoryMapURI; update the rendering in AlgorithmDetailsDialog (the
JSX that creates the <a> blocks using buttonVariants, translate and
ExternalLinkIcon) to first validate/normalize each URL (e.g., use the URL
constructor or a allowlist check) and only render the corresponding anchor when
the value is a well-formed http or https URL; skip rendering the button when
validation fails or the scheme is not http(s). Ensure you reference
algorithm.uri and algorithm.categoryMapURI in the validation logic so the UI
never injects non-http(s) or malformed hrefs.
In `@ui/src/pages/occurrences/occurrences-actions.tsx`:
- Around line 62-81: The memoized agreeParams computed in useMemo references
userInfo?.id but only lists [occurrences] as its dependency, causing stale
results when userInfo changes; update the dependency array for the useMemo that
defines agreeParams (the useMemo block creating agreeParams) to include userInfo
(or userInfo?.id) in addition to occurrences so the memo recomputes when the
user context changes.
- Around line 65-70: The filter in agreeParams uses the wrong predicate and a
plural name: replace the .filter((occurrences) => { ... }) callback with a
singular parameter (e.g., occurrence) and call the same agreement check used
elsewhere: use occurrence.userAgreed(userInfo?.id) (or the equivalent method
used by allAgreed) instead of comparing userInfo.id ===
occurrence.determinationVerifiedBy?.id so the agreeParams list is consistent
with allAgreed; ensure the null-safe access to userInfo?.id when passing into
occurrence.userAgreed.
In `@ui/src/utils/language.ts`:
- Around line 616-617: The tooltip string for STRING.TOOLTIP_SCORE contains a
grammar typo; update the string value used in STRING.TOOLTIP_SCORE to read:
"This is a model derived prediction score, not a real-world probability. Think
of it as a relative metric that will vary based on model calibration and
available training data." — locate the constant STRING.TOOLTIP_SCORE in
ui/src/utils/language.ts and replace the erroneous phrase "vary based model
calibration" with "vary based on model calibration".
---
Nitpick comments:
In `@ui/src/design-system/components/table/table-header/table-header.tsx`:
- Around line 7-14: The TableHeaderProps interface declares an unused sticky?:
boolean prop; remove this property from TableHeaderProps so the code
consistently uses column.sticky (used by BasicTableHeader and
SortableTableHeader) and update any refs to TableHeaderProps if necessary to
rely on column.sticky instead of a separate prop.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d31ccaf-4efa-4fc2-b561-477502778a19
📒 Files selected for processing (19)
ui/src/components/filtering/default-filter-control.tsxui/src/components/filtering/filters/verification-status-filter.tsxui/src/data-services/hooks/processing-services/useProcessingServiceDetails.tsui/src/design-system/components/table/table-header/table-header.module.scssui/src/design-system/components/table/table-header/table-header.tsxui/src/design-system/components/table/table/table.module.scssui/src/design-system/components/table/table/table.tsxui/src/pages/algorithm-details/algorithm-details-dialog.tsxui/src/pages/occurrences/occurrences-actions.tsxui/src/pages/project/capture-sets/populate-capture-set.tsxui/src/pages/project/entities/entities-columns.tsxui/src/pages/project/pipelines/default-pipeline-badge.tsxui/src/pages/project/pipelines/pipelines.tsxui/src/pages/project/processing-services/processing-services-actions.tsxui/src/pages/project/processing-services/processing-services-columns.tsxui/src/pages/session-details/playback/frame/frame.tsxui/src/pages/taxa-list-details/taxa-list-details-columns.tsxui/src/pages/taxa-list-details/taxa-list-details.tsxui/src/utils/language.ts
✅ Files skipped from review due to trivial changes (3)
- ui/src/design-system/components/table/table-header/table-header.module.scss
- ui/src/components/filtering/filters/verification-status-filter.tsx
- ui/src/pages/project/processing-services/processing-services-actions.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- ui/src/design-system/components/table/table/table.tsx
- ui/src/pages/project/capture-sets/populate-capture-set.tsx
- ui/src/pages/taxa-list-details/taxa-list-details.tsx
- ui/src/pages/session-details/playback/frame/frame.tsx
- ui/src/components/filtering/default-filter-control.tsx
- ui/src/data-services/hooks/processing-services/useProcessingServiceDetails.ts
- ui/src/pages/project/pipelines/default-pipeline-badge.tsx
- ui/src/pages/project/entities/entities-columns.tsx
- ui/src/pages/taxa-list-details/taxa-list-details-columns.tsx
annavik
force-pushed
the
chore/ui-cleanup
branch
from
be70b7f to
2cb5687
Compare
annavik
force-pushed
the
chore/ui-cleanup
branch
from
2cb5687 to
476abf7
Compare
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/project/processing-services/processing-services-actions.tsx (1)
18-24:⚠️ Potential issue | 🟡 MinorLocalize the error tooltip copy.
The non-error path uses
translate(...), but the failure state falls back to a hardcoded English string. That will make this control switch languages by state in localized builds. Please move the error message intoSTRINGand translate it here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/project/processing-services/processing-services-actions.tsx` around lines 18 - 24, The tooltip's error branch uses a hardcoded English string; add a new key (e.g., REGISTER_PIPELINES_ERROR) to the STRING enum/object and replace the hardcoded message in the BasicTooltip content with translate(STRING.REGISTER_PIPELINES_ERROR) so both success and error paths use translate(...). Update any relevant test/fixtures if needed and ensure the new STRING key is included in localization resources.
🤖 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/table/table-header/table-header.tsx`:
- Around line 46-48: The header-side sticky class (styles.sticky) is being
toggled in TableHeader (className={classNames(styles.tableHeader,
{[styles.sticky]: column.sticky})}) before the header SCSS actually provides the
same positioning/background rules the body uses; update
ui/src/design-system/components/table/table-header/table-header.module.scss to
mirror the body sticky rules from
ui/src/design-system/components/table/table/table.module.scss (same media query
conditions) — include the positioning (position: sticky), top offset, z-index,
width handling, background color and any border/box-shadow used for visual
separation so header <th> cells align and pin with body cells and the action
column no longer widens unexpectedly; then keep using styles.sticky in the
TableHeader component (also apply the same fix for the other occurrences where
styles.sticky is used).
In `@ui/src/design-system/components/table/table/table.module.scss`:
- Around line 106-112: The .sticky table cell currently has opacity: 0 so when
it becomes position: sticky it allows underlying content to bleed through;
update the CSS for td.sticky (the .sticky class in table.module.scss) to give it
an opaque background (e.g., set a background-color matching the table/toolbar
and remove or avoid transparent opacity) and ensure z-index remains high so the
sticky cell sits above scrolled content.
---
Outside diff comments:
In `@ui/src/pages/project/processing-services/processing-services-actions.tsx`:
- Around line 18-24: The tooltip's error branch uses a hardcoded English string;
add a new key (e.g., REGISTER_PIPELINES_ERROR) to the STRING enum/object and
replace the hardcoded message in the BasicTooltip content with
translate(STRING.REGISTER_PIPELINES_ERROR) so both success and error paths use
translate(...). Update any relevant test/fixtures if needed and ensure the new
STRING key is included in localization resources.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe891ccc-9d17-49b1-957a-b2f0341558ce
📒 Files selected for processing (19)
ui/src/components/filtering/default-filter-control.tsxui/src/components/filtering/filters/verification-status-filter.tsxui/src/data-services/hooks/processing-services/useProcessingServiceDetails.tsui/src/design-system/components/table/table-header/table-header.module.scssui/src/design-system/components/table/table-header/table-header.tsxui/src/design-system/components/table/table/table.module.scssui/src/design-system/components/table/table/table.tsxui/src/pages/algorithm-details/algorithm-details-dialog.tsxui/src/pages/occurrences/occurrences-actions.tsxui/src/pages/project/capture-sets/populate-capture-set.tsxui/src/pages/project/entities/entities-columns.tsxui/src/pages/project/pipelines/default-pipeline-badge.tsxui/src/pages/project/pipelines/pipelines.tsxui/src/pages/project/processing-services/processing-services-actions.tsxui/src/pages/project/processing-services/processing-services-columns.tsxui/src/pages/session-details/playback/frame/frame.tsxui/src/pages/taxa-list-details/taxa-list-details-columns.tsxui/src/pages/taxa-list-details/taxa-list-details.tsxui/src/utils/language.ts
✅ Files skipped from review due to trivial changes (4)
- ui/src/components/filtering/filters/verification-status-filter.tsx
- ui/src/pages/project/pipelines/default-pipeline-badge.tsx
- ui/src/pages/algorithm-details/algorithm-details-dialog.tsx
- ui/src/pages/taxa-list-details/taxa-list-details.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- ui/src/design-system/components/table/table-header/table-header.module.scss
- ui/src/design-system/components/table/table/table.tsx
- ui/src/pages/project/pipelines/pipelines.tsx
- ui/src/data-services/hooks/processing-services/useProcessingServiceDetails.ts
- ui/src/utils/language.ts
- ui/src/pages/occurrences/occurrences-actions.tsx
- ui/src/pages/project/capture-sets/populate-capture-set.tsx
- ui/src/pages/taxa-list-details/taxa-list-details-columns.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
New Features
UI/UX Improvements
Content & Behavior
Localization