Open
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughIntroduces a new AdminPage component and AnnotationServicesAd; migrates many admin views to use AdminPage; adds Scripts docs and expands Projects/Time Tracking docs; small UI tweaks (modal label, team selection API) and minor typing/layout adjustments. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
🧹 Nitpick comments (3)
frontend/javascripts/admin/voxelytics/workflow_list_view.tsx (1)
174-176: Consider adding a documentation link.Other admin pages in this PR include a
descriptionURIprop linking to relevant documentation. If documentation exists for Voxelytics workflows, consider adding it here for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/javascripts/admin/voxelytics/workflow_list_view.tsx` around lines 174 - 176, Add a descriptionURI prop to the AdminPage component instantiation in workflow_list_view.tsx (the AdminPage with title "Voxelytics Workflows") so it matches other admin pages; set descriptionURI to the relevant docs URL for Voxelytics workflows (or to the internal docs route if available). Ensure the prop name is descriptionURI and place it alongside title and description in the AdminPage props so it follows the same pattern as other admin pages.frontend/javascripts/viewer/view/action_bar/user_scripts_modal_view.tsx (1)
87-88: Consider aligning the modal title with the updated button text.The
okTextwas changed to "Run Script" which accurately reflects the action (executing viaeval), but the modal title still says "Add User Script". Consider updating the title to something like "Run User Script" for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/javascripts/viewer/view/action_bar/user_scripts_modal_view.tsx` around lines 87 - 88, Update the modal title to match the action by changing the Modal's title prop from "Add User Script" to "Run User Script" (or similar) so it aligns with okText="Run Script"; locate the Modal in user_scripts_modal_view.tsx (the component rendering the modal with title and okText props) and update the title string accordingly, ensuring any related accessibility labels or test IDs referencing the old title are updated to keep tests and a11y consistent.docs/tasks_projects/scripts.md (1)
3-7: Add an explicit trust/safety warning for script execution.Since scripts execute arbitrary JavaScript, this page should clearly instruct admins to use only trusted sources and review changes before assignment.
Suggested addition
Scripts let you run custom JavaScript logic automatically when annotators open assigned tasks. They are useful for project-specific keyboard shortcuts, UI helpers, and task setup automation. + +> [!WARNING] +> Scripts execute arbitrary JavaScript in the annotator's browser context. +> Only use trusted script sources/owners, and review script updates before assigning them to tasks.Also applies to: 68-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/tasks_projects/scripts.md` around lines 3 - 7, Add a clear trust/safety warning near the top of the "Scripts let you run custom JavaScript logic…" section (the opening paragraph that links to the WEBKNOSSOS Frontend API documentation) instructing admins to run only scripts from trusted sources, to review code changes before assigning scripts to annotators, and to consider restricting script usage to trusted users or environments; also replicate the same warning where the scripts feature is described later in the document (the other scripts description block referenced in the review) so both places contain the explicit safety guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/tasks_projects/projects.md`:
- Line 59: Update the bullet text that currently reads "Expandable rows for
detailed per-user time tracking entries." to hyphenate the compound modifier by
changing "time tracking entries" to "time-tracking entries" so the phrase
becomes "Expandable rows for detailed per-user time-tracking entries." Locate
and edit the exact line containing that sentence in the projects.md content.
In `@frontend/javascripts/admin/scripts/script_list_view.tsx`:
- Around line 163-171: Replace the legacy boolean prop on the Ant Design Space
component by changing the Space usage in script_list_view (the element wrapping
Link and LinkButton around EditOutlined/DeleteOutlined) from <Space vertical
...> to use the modern prop orientation="vertical"; keep other props and
children (Link to={`/scripts/${script.id}/edit`}, LinkButton
onClick={partial(deleteScript, script)} ) unchanged to ensure v6 compatibility.
In `@frontend/javascripts/admin/tasktype/task_type_create_view.tsx`:
- Around line 282-287: In the Select JSX in task_type_create_view.tsx, remove
the standalone prop optionFilterProp="label" and instead pass it inside the
showSearch object (e.g., change to showSearch={{ optionFilterProp: 'label' }}
and keep other showSearch settings as needed) so the Select conforms to Ant
Design v6; update the Select declaration (the component using mode="multiple",
allowClear, placeholder, etc.) to use the new showSearch format and delete the
separate optionFilterProp prop.
---
Nitpick comments:
In `@docs/tasks_projects/scripts.md`:
- Around line 3-7: Add a clear trust/safety warning near the top of the "Scripts
let you run custom JavaScript logic…" section (the opening paragraph that links
to the WEBKNOSSOS Frontend API documentation) instructing admins to run only
scripts from trusted sources, to review code changes before assigning scripts to
annotators, and to consider restricting script usage to trusted users or
environments; also replicate the same warning where the scripts feature is
described later in the document (the other scripts description block referenced
in the review) so both places contain the explicit safety guidance.
In `@frontend/javascripts/admin/voxelytics/workflow_list_view.tsx`:
- Around line 174-176: Add a descriptionURI prop to the AdminPage component
instantiation in workflow_list_view.tsx (the AdminPage with title "Voxelytics
Workflows") so it matches other admin pages; set descriptionURI to the relevant
docs URL for Voxelytics workflows (or to the internal docs route if available).
Ensure the prop name is descriptionURI and place it alongside title and
description in the AdminPage props so it follows the same pattern as other admin
pages.
In `@frontend/javascripts/viewer/view/action_bar/user_scripts_modal_view.tsx`:
- Around line 87-88: Update the modal title to match the action by changing the
Modal's title prop from "Add User Script" to "Run User Script" (or similar) so
it aligns with okText="Run Script"; locate the Modal in
user_scripts_modal_view.tsx (the component rendering the modal with title and
okText props) and update the title string accordingly, ensuring any related
accessibility labels or test IDs referencing the old title are updated to keep
tests and a11y consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b6834540-69d0-4dbb-a59d-1045384392f9
📒 Files selected for processing (21)
docs/tasks_projects/index.mddocs/tasks_projects/projects.mddocs/tasks_projects/scripts.mdfrontend/javascripts/admin/admin_page.tsxfrontend/javascripts/admin/ads/annotation_services_alert.tsxfrontend/javascripts/admin/job/job_list_view.tsxfrontend/javascripts/admin/project/project_create_view.tsxfrontend/javascripts/admin/project/project_list_view.tsxfrontend/javascripts/admin/scripts/script_create_view.tsxfrontend/javascripts/admin/scripts/script_list_view.tsxfrontend/javascripts/admin/statistic/available_tasks_report_view.tsxfrontend/javascripts/admin/statistic/project_progress_report_view.tsxfrontend/javascripts/admin/statistic/time_tracking_overview.tsxfrontend/javascripts/admin/task/task_list_view.tsxfrontend/javascripts/admin/tasktype/task_type_create_view.tsxfrontend/javascripts/admin/tasktype/task_type_list_view.tsxfrontend/javascripts/admin/team/team_list_view.tsxfrontend/javascripts/admin/user/user_list_view.tsxfrontend/javascripts/admin/voxelytics/ai_model_list_view.tsxfrontend/javascripts/admin/voxelytics/workflow_list_view.tsxfrontend/javascripts/viewer/view/action_bar/user_scripts_modal_view.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/javascripts/admin/tasktype/task_type_create_view.tsx (2)
82-85:⚠️ Potential issue | 🟠 MajorEnsure team loading state is cleared on fetch failures.
If
getEditableTeams()throws,isFetchingDatanever flips tofalse, leaving the Team<Select>stuck in loading state.Suggested fix
const teams = useFetch( async () => { - const teams = await getEditableTeams(); - setIsFetchingData(false); - return teams; + try { + return await getEditableTeams(); + } finally { + setIsFetchingData(false); + } }, [], [], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/javascripts/admin/tasktype/task_type_create_view.tsx` around lines 82 - 85, The async loader that calls getEditableTeams() must always clear the fetching flag; wrap the await in a try/finally so setIsFetchingData(false) runs whether getEditableTeams() resolves or throws. Specifically, update the async arrow function that calls getEditableTeams() to use try { const teams = await getEditableTeams(); return teams } finally { setIsFetchingData(false) }, and decide whether to rethrow the error or return an empty array depending on existing error handling.
56-70:⚠️ Potential issue | 🟡 MinorFix user-facing validation typos in magnification errors.
The messages use “then” where “than” is intended (Line 61, Line 69).
Suggested fix
return Promise.reject( - new Error("The minimum magnification needs to be smaller then the maximum mag."), + new Error("The minimum magnification needs to be smaller than the maximum mag."), ); } function isMaximumMagnificationSmallerThenMinRule(value: number | undefined, minMag: number) { if (value && value >= minMag) { return Promise.resolve(); } return Promise.reject( - new Error("The maximum magnification needs to be larger then the minimum mag."), + new Error("The maximum magnification needs to be larger than the minimum mag."), ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/javascripts/admin/tasktype/task_type_create_view.tsx` around lines 56 - 70, The error messages in isMinimumMagnifactionLargerThenMaxRule and isMaximumMagnificationSmallerThenMinRule use the incorrect word "then" instead of "than"; update the Promise.reject error strings to use "than" (e.g., "The minimum magnification needs to be smaller than the maximum mag." and "The maximum magnification needs to be larger than the minimum mag.") so the user-facing validation messages are grammatically correct.
🧹 Nitpick comments (2)
docs/tasks_projects/projects.md (2)
30-31: Standardize UI action formatting for button labels.Line 30 uses “Click Search” while other UI labels are consistently code-formatted. Consider
Click \Search`.` for consistency and scanability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/tasks_projects/projects.md` around lines 30 - 31, Standardize the UI action text by replacing the plain "Click Search" phrase with a code-formatted UI label `Click `Search`` so it matches other button labels; locate the occurrence of the string "Click Search" in projects.md (around the step list) and change it to use inline code formatting for the button name (`Search`), and scan the surrounding list items to ensure other actions follow the same `Click `ButtonName`` pattern for consistency.
51-53: Clarify the filter wording to avoid ambiguity.“Include only tasks from projects and/or all annotations” is hard to parse. Consider naming the exact filter controls/choices as shown in the UI so users can map steps directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/tasks_projects/projects.md` around lines 51 - 53, The phrasing "Include only tasks from projects and/or all annotations" is ambiguous; update the docs to use the exact UI control labels so users can map directly — replace that line with explicit options such as "Project filter: [All projects | Specific project]" and "Annotations filter: [All annotations | Only annotations from selected projects]" (or whatever the UI shows) and ensure the list items match the UI control names shown in the app.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/javascripts/admin/tasktype/task_type_create_view.tsx`:
- Around line 307-333: The "Any" option uses an empty string which can violate
the API type for settings.preferredMode (APIAllowedMode | undefined); in the
Select under FormItem name=["settings","preferredMode"] replace the empty-string
value for the "Any" option with undefined (or remove the value so it maps to
undefined) so the form submits undefined instead of "", and ensure allowClear
behaviour also results in undefined; alternatively, add a submit-time transform
that maps "" to undefined before sending to the API.
---
Outside diff comments:
In `@frontend/javascripts/admin/tasktype/task_type_create_view.tsx`:
- Around line 82-85: The async loader that calls getEditableTeams() must always
clear the fetching flag; wrap the await in a try/finally so
setIsFetchingData(false) runs whether getEditableTeams() resolves or throws.
Specifically, update the async arrow function that calls getEditableTeams() to
use try { const teams = await getEditableTeams(); return teams } finally {
setIsFetchingData(false) }, and decide whether to rethrow the error or return an
empty array depending on existing error handling.
- Around line 56-70: The error messages in
isMinimumMagnifactionLargerThenMaxRule and
isMaximumMagnificationSmallerThenMinRule use the incorrect word "then" instead
of "than"; update the Promise.reject error strings to use "than" (e.g., "The
minimum magnification needs to be smaller than the maximum mag." and "The
maximum magnification needs to be larger than the minimum mag.") so the
user-facing validation messages are grammatically correct.
---
Nitpick comments:
In `@docs/tasks_projects/projects.md`:
- Around line 30-31: Standardize the UI action text by replacing the plain
"Click Search" phrase with a code-formatted UI label `Click `Search`` so it
matches other button labels; locate the occurrence of the string "Click Search"
in projects.md (around the step list) and change it to use inline code
formatting for the button name (`Search`), and scan the surrounding list items
to ensure other actions follow the same `Click `ButtonName`` pattern for
consistency.
- Around line 51-53: The phrasing "Include only tasks from projects and/or all
annotations" is ambiguous; update the docs to use the exact UI control labels so
users can map directly — replace that line with explicit options such as
"Project filter: [All projects | Specific project]" and "Annotations filter:
[All annotations | Only annotations from selected projects]" (or whatever the UI
shows) and ensure the list items match the UI control names shown in the app.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 435a2573-6b5e-465b-ad26-8a156f4b8701
📒 Files selected for processing (3)
docs/tasks_projects/projects.mdfrontend/javascripts/admin/scripts/script_list_view.tsxfrontend/javascripts/admin/tasktype/task_type_create_view.tsx
Comment on lines
+307
to
+333
| <FormItem name={["settings", "preferredMode"]} label="Preferred Mode" hasFeedback> | ||
| <Select | ||
| allowClear | ||
| showSearch={{ optionFilterProp: "label" }} | ||
| style={{ | ||
| width: "100%", | ||
| }} | ||
| options={[ | ||
| { | ||
| required: true, | ||
| value: "", | ||
| label: "Any", | ||
| }, | ||
| ]} | ||
| > | ||
| <Select | ||
| allowClear | ||
| showSearch={{ optionFilterProp: "label" }} | ||
| placeholder="Select a Team" | ||
| style={{ | ||
| width: "100%", | ||
| }} | ||
| loading={isFetchingData} | ||
| options={teams.map((team: APITeam) => ({ | ||
| value: team.id, | ||
| label: `${team.name}`, | ||
| }))} | ||
| /> | ||
| </FormItem> | ||
|
|
||
| <FormItem | ||
| name="description" | ||
| label={ | ||
| <span> | ||
| Description ( | ||
| <a href="https://markdown-it.github.io/" target="_blank" rel="noopener noreferrer"> | ||
| Markdown enabled | ||
| </a> | ||
| ) | ||
| </span> | ||
| } | ||
| hasFeedback | ||
| rules={[ | ||
| { | ||
| required: true, | ||
| value: "orthogonal", | ||
| label: "Orthogonal", | ||
| }, | ||
| { | ||
| value: "oblique", | ||
| label: "Oblique", | ||
| }, | ||
| ]} | ||
| > | ||
| <TextArea rows={10} /> | ||
| </FormItem> | ||
|
|
||
| <FormItem name="tracingType" label="Annotation Type"> | ||
| <RadioGroup> | ||
| <Radio value="skeleton" disabled={isEditingMode}> | ||
| Skeleton | ||
| </Radio> | ||
| <Radio value="volume" disabled={isEditingMode}> | ||
| Volume | ||
| </Radio> | ||
| <Radio value="hybrid" disabled={isEditingMode}> | ||
| Skeleton and Volume | ||
| </Radio> | ||
| </RadioGroup> | ||
| </FormItem> | ||
|
|
||
| <FormItem | ||
| name={["settings", "allowedModes"]} | ||
| label="Allowed Modes" | ||
| hasFeedback | ||
| rules={[ | ||
| { | ||
| required: true, | ||
| value: "flight", | ||
| label: "Flight", | ||
| }, | ||
| ]} | ||
| > | ||
| <Select | ||
| mode="multiple" | ||
| allowClear | ||
| placeholder="Select all Allowed Modes" | ||
| optionFilterProp="label" | ||
| style={{ | ||
| width: "100%", | ||
| }} | ||
| options={[ | ||
| { | ||
| value: "orthogonal", | ||
| label: "Orthogonal", | ||
| }, | ||
| { | ||
| value: "oblique", | ||
| label: "Oblique", | ||
| }, | ||
| { | ||
| value: "flight", | ||
| label: "Flight", | ||
| }, | ||
| ]} | ||
| /> | ||
| </FormItem> | ||
|
|
||
| <FormItem name={["settings", "preferredMode"]} label="Preferred Mode" hasFeedback> | ||
| <Select | ||
| allowClear | ||
| showSearch={{ optionFilterProp: "label" }} | ||
| style={{ | ||
| width: "100%", | ||
| }} | ||
| options={[ | ||
| { | ||
| value: "", | ||
| label: "Any", | ||
| }, | ||
| { | ||
| value: "orthogonal", | ||
| label: "Orthogonal", | ||
| }, | ||
| { | ||
| value: "oblique", | ||
| label: "Oblique", | ||
| }, | ||
| { | ||
| value: "flight", | ||
| label: "Flight", | ||
| }, | ||
| ]} | ||
| /> | ||
| </FormItem> | ||
|
|
||
| <FormItem | ||
| noStyle | ||
| shouldUpdate={(prevValues, curValues) => | ||
| prevValues.tracingType !== curValues.tracingType | ||
| } | ||
| > | ||
| {({ getFieldValue }) => ( | ||
| <div> | ||
| {/* Skeleton-specific */} | ||
| <div | ||
| style={{ | ||
| // These form items are always emitted here and only their visibility | ||
| // is changed, since the values are always needed to create/edit | ||
| // a task type (its schema requires it even though the fields are | ||
| // irrelevant for volume-only tasks). | ||
| display: | ||
| getFieldValue(["tracingType"]) === TracingTypeEnum.volume ? "none" : "block", | ||
| }} | ||
| > | ||
| <FormItem | ||
| name={["settings", "somaClickingAllowed"]} | ||
| label="Settings" | ||
| valuePropName="checked" | ||
| > | ||
| <Checkbox>Allow Single-node-tree mode ("Soma clicking")</Checkbox> | ||
| </FormItem> | ||
|
|
||
| <FormItem name={["settings", "branchPointsAllowed"]} valuePropName="checked"> | ||
| <Checkbox>Allow Branchpoints</Checkbox> | ||
| </FormItem> | ||
| <FormItem name={["settings", "mergerMode"]} valuePropName="checked"> | ||
| <Checkbox>Merger Mode</Checkbox> | ||
| </FormItem> | ||
| </div> | ||
|
|
||
| {/* Volume-specific */} | ||
| <div | ||
| style={{ | ||
| // These form items are always emitted here and only their visibility | ||
| // is changed, since the values are always needed to create/edit | ||
| // a task type (its schema requires it even though the fields are | ||
| // irrelevant for skeleton-only tasks). | ||
| display: | ||
| getFieldValue(["tracingType"]) === TracingTypeEnum.skeleton | ||
| ? "none" | ||
| : "block", | ||
| }} | ||
| /> | ||
| </FormItem> |
Contributor
There was a problem hiding this comment.
preferredMode currently allows an invalid empty-string value.
Line 316 sets value: "" for “Any”, but settings.preferredMode is typed as APIAllowedMode | undefined. Submitting "" can violate the API contract and cause backend validation issues.
Suggested fix
<FormItem name={["settings", "preferredMode"]} label="Preferred Mode" hasFeedback>
<Select
allowClear
+ placeholder="Any"
showSearch={{ optionFilterProp: "label" }}
style={{
width: "100%",
}}
options={[
- {
- value: "",
- label: "Any",
- },
{
value: "orthogonal",
label: "Orthogonal",
},
{
value: "oblique",
label: "Oblique",
},
{
value: "flight",
label: "Flight",
},
]}
/>
</FormItem>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/javascripts/admin/tasktype/task_type_create_view.tsx` around lines
307 - 333, The "Any" option uses an empty string which can violate the API type
for settings.preferredMode (APIAllowedMode | undefined); in the Select under
FormItem name=["settings","preferredMode"] replace the empty-string value for
the "Any" option with undefined (or remove the value so it maps to undefined) so
the form submits undefined instead of "", and ensure allowClear behaviour also
results in undefined; alternatively, add a submit-time transform that maps "" to
undefined before sending to the API.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/javascripts/admin/statistic/time_tracking_overview.tsx (1)
241-283: Make the filter bar wrap to avoid overflow on narrower widths.Lines 241–283 place several wide controls in a non-wrapping
Space, which can push content horizontally. Adding wrap keeps the new AdminPage layout responsive.♻️ Suggested diff
- filters={ - <Space> + filters={ + <Space wrap> <ProjectAndAnnotationTypeDropdown setSelectedProjectIds={setSelectedProjectIds} selectedProjectIds={selectedProjectIds} setSelectedAnnotationType={setSelectedTypes} selectedAnnotationType={selectedTypes} selectedAnnotationState={selectedState} setSelectedAnnotationState={setSelectedState} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/javascripts/admin/statistic/time_tracking_overview.tsx` around lines 241 - 283, The toolbar uses an Ant Design <Space> that prevents wrapping and causes horizontal overflow; update the container around ProjectAndAnnotationTypeDropdown, the Select, and the RangePicker to allow wrapping (e.g., enable the Space wrap behavior or switch to a flex container with flexWrap:'wrap' and appropriate alignment/spacing) so the controls reflow on narrow widths—adjust the <Space> (wrapping) or replace it with a responsive flex/Row wrapper and keep the existing props/state handlers (setSelectedProjectIds, selectedProjectIds, setSelectedTypes, selectedTypes, selectedState, setSelectedState, setSelectedTeams, startDate, endDate, setStartDate, setEndeDate) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/javascripts/admin/statistic/project_progress_report_view.tsx`:
- Around line 175-207: Rows where item.totalInstances === 0 are treated as
finished (showing "100%") and passed as all-zero values into the
StackedBarChart, which can produce invalid rendering; update the render logic in
the Column renderer for finishedInstances to first check item.totalInstances ===
0 and render an explicit "0%" or a neutral Badge (and render StackedBarChart
with a guard/early-return or a special empty-state) instead of computing
Math.floor((100 * finishedInstances) / item.totalInstances) or passing a,b,c
when total is zero. Locate the Column with title Badge/finished and the render
function that returns colSpan/children (uses finishedInstances, activeInstances,
pendingInstances and StackedBarChart) and add the zero-total branch there.
---
Nitpick comments:
In `@frontend/javascripts/admin/statistic/time_tracking_overview.tsx`:
- Around line 241-283: The toolbar uses an Ant Design <Space> that prevents
wrapping and causes horizontal overflow; update the container around
ProjectAndAnnotationTypeDropdown, the Select, and the RangePicker to allow
wrapping (e.g., enable the Space wrap behavior or switch to a flex container
with flexWrap:'wrap' and appropriate alignment/spacing) so the controls reflow
on narrow widths—adjust the <Space> (wrapping) or replace it with a responsive
flex/Row wrapper and keep the existing props/state handlers
(setSelectedProjectIds, selectedProjectIds, setSelectedTypes, selectedTypes,
selectedState, setSelectedState, setSelectedTeams, startDate, endDate,
setStartDate, setEndeDate) intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2929bbce-7541-48ed-8335-f9789b1fefab
📒 Files selected for processing (6)
frontend/javascripts/admin/statistic/available_tasks_report_view.tsxfrontend/javascripts/admin/statistic/project_and_annotation_type_dropdown.tsxfrontend/javascripts/admin/statistic/project_progress_report_view.tsxfrontend/javascripts/admin/statistic/team_selection_form.tsxfrontend/javascripts/admin/statistic/time_tracking_overview.tsxfrontend/javascripts/dashboard/dataset/team_selection_component.tsx
💤 Files with no reviewable changes (1)
- frontend/javascripts/admin/statistic/team_selection_form.tsx
Comment on lines
+175
to
+207
| finishedInstances === item.totalInstances ? ( | ||
| <Badge | ||
| count="100%" | ||
| style={{ | ||
| backgroundColor: colors.finished, | ||
| }} | ||
| /> | ||
| ) : ( | ||
| <span>{Math.floor((100 * finishedInstances) / item.totalInstances)} %</span> | ||
| ) | ||
| } | ||
| /> | ||
| <Column | ||
| title={ | ||
| <Badge | ||
| count="Finished" | ||
| style={{ | ||
| background: colors.finished, | ||
| }} | ||
| /> | ||
| } | ||
| dataIndex="finishedInstances" | ||
| sorter={compareBy<APIProjectProgressReport>((project) => project.finishedInstances)} | ||
| render={(_text, item: APIProjectProgressReport) => ({ | ||
| props: { | ||
| colSpan: 3, | ||
| }, | ||
| children: ( | ||
| <StackedBarChart | ||
| a={item.finishedInstances} | ||
| b={item.activeInstances} | ||
| c={item.pendingInstances} | ||
| /> |
Contributor
There was a problem hiding this comment.
Handle zero-instance projects explicitly in progress/chart rendering.
At Lines 175–207, rows with totalInstances === 0 can display misleading progress (100%) and may feed all-zero values into StackedBarChart, which can render invalid widths. Add an explicit zero-total path before percentage/chart rendering.
🐛 Suggested diff
<Column
title="Progress"
key="progress"
dataIndex="finishedInstances"
width={100}
sorter={compareBy<APIProjectProgressReport>(
({ finishedInstances, totalInstances }) => finishedInstances / totalInstances,
)}
- render={(finishedInstances, item) =>
- finishedInstances === item.totalInstances ? (
+ render={(finishedInstances, item) => {
+ if (item.totalInstances === 0) {
+ return <span>0 %</span>;
+ }
+ return finishedInstances === item.totalInstances ? (
<Badge
count="100%"
style={{
backgroundColor: colors.finished,
}}
/>
) : (
<span>{Math.floor((100 * finishedInstances) / item.totalInstances)} %</span>
- )
- }
+ );
+ }}
/>
@@
dataIndex="finishedInstances"
sorter={compareBy<APIProjectProgressReport>((project) => project.finishedInstances)}
render={(_text, item: APIProjectProgressReport) => ({
props: {
colSpan: 3,
},
children: (
- <StackedBarChart
- a={item.finishedInstances}
- b={item.activeInstances}
- c={item.pendingInstances}
- />
+ item.totalInstances === 0 ? (
+ <span>0</span>
+ ) : (
+ <StackedBarChart
+ a={item.finishedInstances}
+ b={item.activeInstances}
+ c={item.pendingInstances}
+ />
+ )
),
})}
/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/javascripts/admin/statistic/project_progress_report_view.tsx` around
lines 175 - 207, Rows where item.totalInstances === 0 are treated as finished
(showing "100%") and passed as all-zero values into the StackedBarChart, which
can produce invalid rendering; update the render logic in the Column renderer
for finishedInstances to first check item.totalInstances === 0 and render an
explicit "0%" or a neutral Badge (and render StackedBarChart with a
guard/early-return or a special empty-state) instead of computing
Math.floor((100 * finishedInstances) / item.totalInstances) or passing a,b,c
when total is zero. Locate the Column with title Badge/finished and the render
function that returns colSpan/children (uses finishedInstances, activeInstances,
pendingInstances and StackedBarChart) and add the zero-total branch there.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx (1)
239-239:⚠️ Potential issue | 🟡 MinorReplace deprecated
dropdownMatchSelectWidthwithpopupMatchSelectWidth.The
dropdownMatchSelectWidthprop was renamed topopupMatchSelectWidthin Ant Design v5+. Update the prop name to ensure the width constraint is applied correctly in v6.🔧 Suggested fix
<Select mode="multiple" allowClear autoFocus style={{ minWidth: 400 }} - dropdownMatchSelectWidth={false} + popupMatchSelectWidth={false} placeholder="Please select" onChange={handleChange} options={options} value={selectedOrganizationIds} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx` at line 239, Replace the deprecated Select prop dropdownMatchSelectWidth with popupMatchSelectWidth in the ai_model_list_view.tsx JSX (look for the Select component usage or the AIModelListView render/return where dropdownMatchSelectWidth={false} is set); change the prop name only (keep the value as-is, e.g., popupMatchSelectWidth={false}) to match Ant Design v5+/v6 API so the width behavior is applied correctly.
🧹 Nitpick comments (3)
frontend/javascripts/admin/statistic/time_tracking_overview.tsx (1)
218-228: Consider removing extraneous whitespace in summary cells.The
{" "}patterns around values in summary cells appear to add unnecessary padding that could be handled via CSS if needed.♻️ Suggested cleanup
<Table.Summary.Cell index={2} align="right"> - {" "} - {totalNumberOfTasksAndAnnotations}{" "} + {totalNumberOfTasksAndAnnotations} </Table.Summary.Cell> <Table.Summary.Cell index={3} align="right"> {formatMilliseconds(totalTimeMs / totalNumberOfTasksAndAnnotations)} </Table.Summary.Cell> <Table.Summary.Cell index={4} align="right"> - {" "} - {formatMilliseconds(totalTimeMs)} + {formatMilliseconds(totalTimeMs)} </Table.Summary.Cell>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/javascripts/admin/statistic/time_tracking_overview.tsx` around lines 218 - 228, Remove the extraneous JSX whitespace tokens surrounding values in the summary cells: in the Table.Summary.Cell instances that render totalNumberOfTasksAndAnnotations, formatMilliseconds(totalTimeMs / totalNumberOfTasksAndAnnotations), and formatMilliseconds(totalTimeMs) remove the {" "} entries so the cells only render the value; rely on CSS/padding for spacing instead of literal JSX spaces.frontend/javascripts/admin/tasktype/task_type_list_view.tsx (1)
113-121: Type mismatch betweenonPressEnterandhandleSearchhandler.The
@ts-expect-errorcomment masks a genuine type incompatibility.onPressEnterexpects aKeyboardEventHandlerbuthandleSearchis typed forChangeEvent<HTMLInputElement>. While this works at runtime (both events havetarget.value), the type suppression is incorrect.Consider either:
- Creating a separate handler for
onPressEnterthat doesn't require the event, or- Removing
onPressEntersinceonChangealready updates the search query in real-time♻️ Suggested fix
search={ <Search allowClear - // `@ts-expect-error` ts-migrate(2322) FIXME: Type '(event: React.ChangeEvent<HTMLInputElement>)... Remove this comment to see the full error message - onPressEnter={handleSearch} onChange={handleSearch} value={searchQuery} /> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/javascripts/admin/tasktype/task_type_list_view.tsx` around lines 113 - 121, The onPressEnter prop is incorrectly assigned handleSearch (a ChangeEvent<HTMLInputElement> handler) causing a TS type mismatch; either remove the onPressEnter prop if realtime onChange is sufficient, or add a new keyboard-specific handler (e.g., handlePressEnter) that matches KeyboardEventHandler<HTMLInputElement> and delegates to the same search logic (or calls handleSearch's core logic without requiring a ChangeEvent). Update the Search usage to pass the new handlePressEnter for onPressEnter and keep handleSearch for onChange, and remove the `@ts-expect-error` comment.frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx (1)
22-22: Consider extractingSearchdestructuring outside the component.Destructuring
{ Search } = Inputinside the component body creates a new reference on each render. Move this outside the component or useInput.Searchdirectly in the JSX.♻️ Suggested refactor
+const { Search } = Input; + export default function AiModelListView() { - const { Search } = Input; const activeUser = useWkSelector((state) => enforceActiveUser(state.activeUser));Or use
Input.Searchdirectly at line 67:- <Search + <Input.Search allowClear onChange={(event) => setSearchQuery(event.target.value)} value={searchQuery} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx` at line 22, The destructuring const { Search } = Input is done inside the component, causing a new reference each render; move that destructure to module scope (above the component) or replace all uses of Search in the JSX with Input.Search so the reference is stable; locate the Search usage and the component in ai_model_list_view.tsx (symbols: Search, Input, and the component function) and either hoist const { Search } = Input to the top of the file or update JSX to use Input.Search.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/javascripts/admin/statistic/time_tracking_detail_view.tsx`:
- Around line 27-31: The return type and local arrays use the removed global JSX
namespace (JSX.Element); update the file to import a proper React type and
replace occurrences: add an import type like `import type { ReactElement } from
"react"` (or `import type { JSX } from "react"`) and change the function's
return annotation and the local variables `taskRows` and `annotationRows` from
`JSX.Element[]` to `ReactElement[]` (or `JSX.Element[]` → `JSX.Element[]` with
the new import), and apply the same replacement pattern to the other listed
files (color_picker.tsx, layer_selection.tsx, mag_selection.tsx,
metadata_table.tsx, download_modal_view.tsx, brush_presets.tsx,
tool_buttons.tsx, bounding_box_selection.tsx,
bounding_box_selection_form_item.tsx, command_palette.tsx) so the code uses an
explicit imported React type instead of the global JSX namespace.
---
Outside diff comments:
In `@frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx`:
- Line 239: Replace the deprecated Select prop dropdownMatchSelectWidth with
popupMatchSelectWidth in the ai_model_list_view.tsx JSX (look for the Select
component usage or the AIModelListView render/return where
dropdownMatchSelectWidth={false} is set); change the prop name only (keep the
value as-is, e.g., popupMatchSelectWidth={false}) to match Ant Design v5+/v6 API
so the width behavior is applied correctly.
---
Nitpick comments:
In `@frontend/javascripts/admin/statistic/time_tracking_overview.tsx`:
- Around line 218-228: Remove the extraneous JSX whitespace tokens surrounding
values in the summary cells: in the Table.Summary.Cell instances that render
totalNumberOfTasksAndAnnotations, formatMilliseconds(totalTimeMs /
totalNumberOfTasksAndAnnotations), and formatMilliseconds(totalTimeMs) remove
the {" "} entries so the cells only render the value; rely on CSS/padding for
spacing instead of literal JSX spaces.
In `@frontend/javascripts/admin/tasktype/task_type_list_view.tsx`:
- Around line 113-121: The onPressEnter prop is incorrectly assigned
handleSearch (a ChangeEvent<HTMLInputElement> handler) causing a TS type
mismatch; either remove the onPressEnter prop if realtime onChange is
sufficient, or add a new keyboard-specific handler (e.g., handlePressEnter) that
matches KeyboardEventHandler<HTMLInputElement> and delegates to the same search
logic (or calls handleSearch's core logic without requiring a ChangeEvent).
Update the Search usage to pass the new handlePressEnter for onPressEnter and
keep handleSearch for onChange, and remove the `@ts-expect-error` comment.
In `@frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx`:
- Line 22: The destructuring const { Search } = Input is done inside the
component, causing a new reference each render; move that destructure to module
scope (above the component) or replace all uses of Search in the JSX with
Input.Search so the reference is stable; locate the Search usage and the
component in ai_model_list_view.tsx (symbols: Search, Input, and the component
function) and either hoist const { Search } = Input to the top of the file or
update JSX to use Input.Search.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4ad115e6-9086-40b9-8f02-0ab57ba1d6a4
📒 Files selected for processing (9)
frontend/javascripts/admin/job/job_list_view.tsxfrontend/javascripts/admin/project/project_list_view.tsxfrontend/javascripts/admin/statistic/available_tasks_report_view.tsxfrontend/javascripts/admin/statistic/project_progress_report_view.tsxfrontend/javascripts/admin/statistic/time_tracking_detail_view.tsxfrontend/javascripts/admin/statistic/time_tracking_overview.tsxfrontend/javascripts/admin/tasktype/task_type_list_view.tsxfrontend/javascripts/admin/team/team_list_view.tsxfrontend/javascripts/admin/voxelytics/ai_model_list_view.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/javascripts/admin/team/team_list_view.tsx
frontend/javascripts/admin/statistic/time_tracking_detail_view.tsx
Outdated
Show resolved
Hide resolved
philippotto
requested changes
Mar 17, 2026
Member
There was a problem hiding this comment.
nice UI overhaul and clean up 👍
some things:
- create script page looks weird:
- shadow without margin (not sure if the shadow makes sense there at all)
- very wide text fields
- create project page and create task type page, too
- tasks page layout could be improved a bit. I highlighted the (imo) too-big whitespace and the too-wide fields
- create task page still has old style
- the settings button in the project progress view does nothing for me
Comment on lines
+6
to
+11
| function normalizeTitle(title: ReactNode) { | ||
| if (typeof title !== "string") { | ||
| return title; | ||
| } | ||
| return title.toUpperCase(); | ||
| } |
Member
There was a problem hiding this comment.
I'm not a big fan of capslock case here. if you still want to keep it, I'd vote for additional css like font-weight: 100; letter-spacing: 6px; to make it less agressive. by the way, one could also use text-transform: uppercase; instead of the string modification.
Member
Author
There was a problem hiding this comment.
I added a textTransform.
Regarding font-weight and letter-spacing: This is the default antd-style for headings (<Typography.Title>). I don't want to overwrite that otherwise we lose styling, consistency all over the app again.
frontend/javascripts/admin/statistic/project_and_annotation_type_dropdown.tsx
Outdated
Show resolved
Hide resolved
Comment on lines
+107
to
+109
| if (!Array.isArray(selectedTeam) && selectedTeam != null) { | ||
| handleTeamChange(selectedTeam); | ||
| } |
Member
There was a problem hiding this comment.
i wasn't able to clear the team again. not sure if it worked before but this line probably prevents it.
Member
Author
There was a problem hiding this comment.
I am not sure what you mean. The table is empty be default unless you select a team. There is not reason "clear" (unselect) a team to show an empty table again.
| return isLoading ? null : ( | ||
| <Alert title="Add more teams" description={teamMessage} type="info" showIcon /> | ||
| ); | ||
| } |
Member
There was a problem hiding this comment.
there's no replacement for this, right? wouldn't it fit well into the alerts prop for the admin page component?
Member
Author
There was a problem hiding this comment.
Well, the informational part has been moved to the page description part including a link to the docs. Having another link to trigger the TeamCreationModal in addition to the primary button did not add much value.
philippotto
reviewed
Mar 17, 2026
| Notes: | ||
|
|
||
| - The view refreshes automatically in regular intervals and can also be refreshed manually. | ||
| - You can re-open the team filter panel at any time via the settings button. |
Member
There was a problem hiding this comment.
ah ok, then the button does something, but it won't hide the team selection UI again. either, the button should get disabled then or it should toggle the visibility.
Member
Author
There was a problem hiding this comment.
Context: Since this report was displayed on a big monitor as a dashboard in the lab, we added the ability to hide the team selection filter a long time ago.
philippotto
reviewed
Mar 17, 2026
docs/tasks_projects/projects.md
Outdated
Comment on lines
+57
to
+61
| - Per-user totals for annotation/task count and tracked time. | ||
| - Average time per task/annotation. | ||
| - Expandable rows for detailed per-user time-tracking entries. | ||
| - CSV export for the current overview. | ||
| - Per-user CSV export of detailed time spans. |
Member
There was a problem hiding this comment.
these are not sentences which is why I'd omit the full stop. also potentially lower-case the first word
docs/tasks_projects/scripts.md
Outdated
|
|
||
| ## Create a Script | ||
|
|
||
| 1. Open the `Scripts` screen in the administration section. |
Member
There was a problem hiding this comment.
do we typically use the word screen in the UI? I'd rather use "page".
Member
Author
There was a problem hiding this comment.
screen: 76x (including "screenshot", and screen aka computer monitor)
page: 120x
I have changed a few other pages as well
docs/tasks_projects/scripts.md
Outdated
Comment on lines
+25
to
+34
| (function () { | ||
| const toggleColor = () => { | ||
| const layer = window.webknossos.getCurrentLayer(); | ||
| if (layer) { | ||
| layer.setColor(layer.getColor() === "red" ? "blue" : "red"); | ||
| } | ||
| }; | ||
|
|
||
| window.webknossos.registerKeyboardShortcut("t", toggleColor); | ||
| })(); |
Member
There was a problem hiding this comment.
I don't think that this is a valid script.
Member
Author
There was a problem hiding this comment.
got point. I am meant include a simple example but then lost track. I updated it.
| ## Best Practices | ||
|
|
||
| - Keep scripts focused and task-specific. | ||
| - Version changes in your Gist history. |
Member
There was a problem hiding this comment.
how about linking github gist in case somebody doesn't know it?
Member
Author
There was a problem hiding this comment.
done, at the top of the page for the first mention of gists
docs/tasks_projects/scripts.md
Outdated
| 2. Click `Add Script`. | ||
| 3. Fill in: | ||
| - `Script Name`: Human-readable label shown in selectors. | ||
| - `Gist URL`: URL of the script source, a public GitHub Gist that your setup can access. |
Member
There was a problem hiding this comment.
does it need to be a gist url? it's not clear whether the second part of the sentence refines the first or whether it is an alternative.
…ate-admin-page-design
…pe_dropdown.tsx Co-authored-by: Philipp Otto <philippotto@users.noreply.github.com>
…ebknossos into update-admin-page-design
…ate-admin-page-design
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR updates and unifies the admin page UI design.
<AdminListPage>component that layouts title, actions, search and the child contentURL of deployed dev instance (used for testing):
Steps to test:
Issues:
(Please delete unneeded items, merge only when none are left open)
$PR_NUMBER.mdfile inunreleased_changesor use./tools/create-changelog-entry.py)