Update FE to use wording "capture sets" instead of "collections" #1127
Conversation
…ctions" where possible
✅ 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces "Collections" with "CaptureSets" across routes, hooks, models, components, filters, translations, and forms; removes collection-detail redirect/component and collection-specific hooks; adds project-scoped filters with tooltip/linking to capture-sets and updates UI labels from images/collections → captures/capture-sets. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
ui/src/utils/language.ts (3)
504-505:⚠️ Potential issue | 🟡 Minor
MESSAGE_PROCESS_NOW_TOOLTIPstill references "image" instead of "capture".This tooltip text reads "Process this single image with presets" — should likely be updated to "capture" for consistency with the rest of the rename.
Proposed fix
- [STRING.MESSAGE_PROCESS_NOW_TOOLTIP]: - 'Process this single image with presets', + [STRING.MESSAGE_PROCESS_NOW_TOOLTIP]: + 'Process this single capture with presets',
416-416:⚠️ Potential issue | 🟡 Minor
FIELD_LABEL_SAMPLE_CAPTURESstill maps to'Sample images'.The key name references "captures" but the displayed string says "images". Should this be "Sample captures" for consistency?
486-487:⚠️ Potential issue | 🟡 MinorPre-existing grammar issue: "must smaller" → "must be smaller".
Not part of this PR's scope, but worth noting: the string reads "The image must smaller than" — it should be "The image must be smaller than".
Proposed fix
[STRING.MESSAGE_IMAGE_SIZE]: - 'The image must smaller than {{value}} {{unit}}.', + 'The image must be smaller than {{value}} {{unit}}.',ui/src/utils/useFilters.ts (1)
72-75:⚠️ Potential issue | 🟡 Minor"Source image" labels not updated to "Capture".
The filters at lines 73 and 109 still use the hardcoded label
'Source image'rather thantranslate(STRING.FIELD_LABEL_CAPTURE). Given the PR's goal of replacing "source image" with "capture" in UI text, these should likely be updated for consistency.Suggested fix
{ - label: 'Source image', + label: translate(STRING.FIELD_LABEL_CAPTURE), field: 'detections__source_image', // This is for viewing Occurrences by source image. `@TODO`: Can we update this key to "source_image" to streamline? },{ - label: 'Source image', + label: translate(STRING.FIELD_LABEL_CAPTURE), field: 'source_image_single', // This is for viewing Jobs by source image. `@TODO`: Can we update this key to "source_image" to streamline? },Also applies to: 108-111
ui/src/pages/job-details/job-details-form/job-details-form.tsx (1)
152-155:⚠️ Potential issue | 🟡 MinorPre-existing bug:
config.pipelineused forstartNowcontroller.Line 155 passes
config.pipelineas the config for thestartNowfield. This should likely beconfig.startNow. Not introduced by this PR, but worth noting.Proposed fix
<FormController name="startNow" control={control} - config={config.pipeline} + config={config.startNow} render={({ field }) => (
🤖 Fix all issues with AI agents
In `@ui/src/pages/captures/upload-images-dialog/upload-images-dialog.tsx`:
- Line 293: In the upload summary of the UploadImagesDialog component locate the
InputValue usage that currently reads InputValue label="Images"
value={images.length} and change the label prop to "Captures" so it reads
label="Captures" (preserving the value={images.length} and any surrounding
structure); ensure no other occurrences of the old "Images" label remain in the
component.
In `@ui/src/pages/project/capture-sets/capture-sets.tsx`:
- Around line 51-58: Update the stale inline comment inside the useEffect that
checks captureSets — change the wording from "collection" to "capture set" so it
reads something like "If any capture set has a job in progress, we want to poll
the endpoint so we can show job updates"; locate the comment near the useEffect
that references captureSets and setPoll and replace the phrase accordingly.
In `@ui/src/utils/constants.ts`:
- Around line 20-21: The exported route constant CAPTURE_SETS was changed to
`/projects/${params.projectId}/capture-sets` but there is no redirect from the
old `/collections` path; update the app router to add a redirect from
`/projects/:projectId/collections` to the new CAPTURE_SETS route (use the same
projectId param and preserve query/search/hash) so bookmarked/shared URLs
continue to work—locate where routes are defined/registered in the router and
add a redirect entry that programmatically resolves to CAPTURE_SETS (or uses
history.replace/navigation redirect) for backward compatibility.
In `@ui/src/utils/useFilters.ts`:
- Around line 23-24: Update the inline TODO comment in the useFilters hook where
the filter with field 'source_image_collection' is defined: fix the typo by
adding the missing word "to" so the comment reads "...Can we update this key to
"capture_set_id" to streamline?"—look for the filter object referencing field
'source_image_collection' in the useFilters.ts file (the hook/function
useFilters) and correct the comment text.
🧹 Nitpick comments (5)
ui/src/pages/captures/upload-images-dialog/select-images-section/select-images-section.tsx (1)
46-46: Leftoverstyles.collectionCSS class name.The className still references
styles.collectionwhile the rest of the PR renames "collection" → "capture set". Consider renaming this CSS class for consistency, though it's not user-facing.ui/src/pages/project/entities/details-form/collection-details-form.tsx (1)
128-135: Consider renamingCollectionDetailsFormand related types toCaptureSetDetailsForm.The component (
CollectionDetailsForm), its type (CollectionFormValues), and the filename (collection-details-form.tsx) still use the old "collection" terminology. This is internal-only so it's not urgent, but it would complete the rename for consistency.ui/src/pages/project/sidebar/useSidebarSections.tsx (1)
138-141: Unused expression — result is computed and discarded.This
.map().flat().find()chain produces a value that is never assigned or returned. It appears to be dead code (likely a leftover from a refactor). Not introduced by this PR, but worth cleaning up.🧹 Remove dead code
- sidebarSections - .map(({ items }) => items) - .flat() - .find((item) => !!matchPath(item.path, location.pathname)) - return { sidebarSections, activeItem }ui/src/data-services/models/capture-set.ts (1)
26-37:currentJobmutates_jobsarray via in-placesort.Pre-existing issue:
this._jobs.sort(...)mutates thereadonlyarray on every access tocurrentJob. Consider using[...this._jobs].sort(...)orthis._jobs.toSorted(...)to avoid side effects.Proposed fix
- return this._jobs.sort((j1: Job, j2: Job) => { + return [...this._jobs].sort((j1: Job, j2: Job) => {ui/src/pages/project/capture-sets/populate-capture-set.tsx (1)
20-20: Hardcoded error string — consider usingtranslate()for consistency with i18n.Other UI strings in this file (e.g.,
translate(STRING.POPULATE)on line 31) go through the translation system. This error tooltip is a raw English string.
ui/src/pages/captures/upload-images-dialog/upload-images-dialog.tsx
Outdated
Show resolved
Hide resolved
|
Exciting! |
|
what could be a simple way to link between the Captures view and Capture Sets? something for now until the layout is reworked? |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ui/src/app.tsx`:
- Around line 112-113: Add a redirect Route for the removed "collections/:id"
path so bookmarked links don't hit NotFound: in the router where <Route
path="capture-sets" element={<CaptureSets />} /> and <Route path="collections"
element={<Collections />} /> are defined, add a route matching "collections/:id"
that redirects (via react-router Navigate) to the new listing (e.g.,
"capture-sets" or "collections") instead of rendering CollectionDetails or
falling through to NotFound; reference the "collections/:id" path, the
CaptureSets/Collections components, and the NotFound behavior when implementing
the redirect.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ui/src/utils/useFilters.ts`:
- Around line 158-160: The code casts projectId from useParams() to string when
calling AVAILABLE_FILTERS(projectId as string), which can propagate "undefined"
into routes (e.g., APP_ROUTES.CAPTURE_SETS) if the hook is used outside a route
with :projectId; change the logic in useFilters so it does not force-cast
projectId — check for projectId existence first and either return an
empty/fallback filters list or build AVAILABLE_FILTERS only when projectId is
defined, and update any places that use the resulting info properties to be
conditional on projectId (refer to useParams, projectId, AVAILABLE_FILTERS and
APP_ROUTES.CAPTURE_SETS to locate the change).
🧹 Nitpick comments (3)
ui/src/utils/useFilters.ts (1)
160-160: Typo:avaibleFilters→availableFilters.This variable name is misspelled on lines 160, 162, 185, and 198. Consider renaming for readability.
✏️ Proposed fix
- const avaibleFilters = AVAILABLE_FILTERS(projectId as string) + const availableFilters = AVAILABLE_FILTERS(projectId as string) - const _filters = avaibleFilters.map(({ field, ...rest }) => { + const _filters = availableFilters.map(({ field, ...rest }) => { - if (avaibleFilters.some((filter) => filter.field === field)) { + if (availableFilters.some((filter) => filter.field === field)) { - if (avaibleFilters.some((filter) => filter.field === field)) { + if (availableFilters.some((filter) => filter.field === field)) {ui/src/components/filtering/filter-control.tsx (2)
113-138:FilterInfocomponent: consider hoistingTooltip.Provider.Each
FilterInfoinstance wraps itself in its own<Tooltip.Provider>. If multiple capture-set filters render on the same page (e.g., on the occurrences or jobs view), you'll have redundant providers. This works correctly, but a singleTooltip.Providerhigher in the tree (or at the app level) would be slightly cleaner and allow shareddelayDurationconfiguration.This is a minor suggestion and not blocking.
125-128: Avoid!importantoverride on the link styling.The
!w-autoTailwind class uses the!importantmodifier to override width frombuttonVariants. If the variant's width is coming from a default you don't want, consider customizing the variant instead, or using a more specific class composition. This is fine for now but can become fragile if variant styles change.
Yes, let's find some way to also connect these views sooner than later! I got a bit focused on the individual views and forgot about this 🙃 I gave this some thinking... One idea could be we apply this UI pattern for filtering controls in general (tooltip with some info and a link to configure): In the capture view, it could look like this. Since the code for filtering controls is quite generalized, we could control all this from the big config object and easily apply the same pattern for other filters as well. We could also reuse the tooltip strings we are using for the main views. Example: I pushed a commit to test this idea for capture sets only to begin with! Let me know what you think? Maybe on the capture page specifically we could also consider a more explicit link to the capture sets page. |
2 participants
Summary
In this PR, we change the wording "collections" to "capture sets". We also make sure to use the term "capture" instead of the more generic "image" in other places in UI, when referring to camera trap source images. We also clarify both the term "Capture" and "Capture set" with tooltips (see screenshots).
List of Changes
Detailed Description
How to Test the Changes
Examples of views affected:
Screenshots
Deployment Notes
No BE deploy needed. These changes only concerns the FE code.
Summary by CodeRabbit
Refactor
UI
New Features