Fix task creation alignment #9142
Conversation
📝 WalkthroughWalkthroughReplaces lodash utilities with native/alternative helpers, migrates inline layout/styling to Ant Design components (Alert, Flex, Typography), refactors task creation object construction using omit/omitBy equivalents, and adjusts Row alignments/margins to fix button alignment in the task creation form. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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 |
| <Alert | ||
| showIcon | ||
| type="warning" | ||
| title="There were warnings during task creation" |
There was a problem hiding this comment.
title prop is already antd v6. Should be message for v5.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/task/task_create_form_view.tsx (2)
109-123: Fix impossible length check indownloadTasksAsCSV
tasks.lengthis never< 0, so this guard is effectively dead and the function will happily compute a filename even for an empty array (which can lead toMath.max(...[])being called). Use<= 0instead to short‑circuit on empty input.export function downloadTasksAsCSV(tasks: Array<APITask>) { - if (tasks.length < 0) { + if (tasks.length <= 0) { return; } @@ - const allProjectNames = uniq(tasks.map((task) => task.projectName)).join("_"); + const allProjectNames = uniq(tasks.map((task) => task.projectName)).join("_");
360-407: EnsurenmlFilesis omitted from the NML task payload to be consistent with other branchesIn the NML creation branch,
nmlFilesis not being omitted from thenewTaskpayload:const newTask: NewNmlTask = { ...omit(formValues, "baseAnnotation"), boundingBox, };This is inconsistent with the update branch and non-NML creation branch, both of which omit both
"nmlFiles"and"baseAnnotation". SincenmlFilesare extracted and passed separately asbatchOfNmlstocreateTaskFromNML, they should not be included in the payload.Apply the same omit pattern:
- const newTask: NewNmlTask = { - ...omit(formValues, "baseAnnotation"), - boundingBox, - }; + const newTask: NewNmlTask = { + ...omit(formValues, "nmlFiles", "baseAnnotation"), + boundingBox, + };
🧹 Nitpick comments (2)
frontend/javascripts/admin/task/task_create_form_view.tsx (2)
251-259: Create/Reload resource button layout changesAdding
marginBottom: "var(--ant-form-item-margin-bottom)"on theColwrappers and using a fixedflex="40px"for the reload icon button should align these controls with neighboring form items and address the button misalignment issue.As an optional future cleanup, you could wrap these in
Form.Itemwithlayout="inline"(supported per recent antd versions) to let the form system manage vertical spacing instead of relying on inline margins. Based on learnings, this would leverageForm.Item’s per‑itemlayoutcontrol.Also applies to: 261-275
470-481: Validator logic usingisEqualontaskResponse.statusUsing
isEqualto checktaskResponse.statusagainst{ pending: 0, active: 0, finished: 1 }before auto‑fillingdatasetName/datasetIdis reasonable and keeps the intent explicit. Just ensure that this “exactly one finished, none pending/active” condition really matches the allowed base‑annotation tasks; otherwise, this could be a bit strict if more finished instances should still be acceptable.Also applies to: 484-503
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/javascripts/admin/task/task_create_form_view.tsx(22 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: hotzenklotz
Repo: scalableminds/webknossos PR: 8849
File: frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/collapsible_split_merger_evaluation_settings.tsx:39-51
Timestamp: 2025-08-14T14:12:03.293Z
Learning: Since Ant Design version 5.18, Form.Item components support the layout prop (layout="horizontal", layout="vertical", layout="inline"), allowing individual form items to override the parent Form's layout setting. This enables mixed layouts within a single form and was added as a new API feature in v5.18 according to the official changelog.
Learnt from: hotzenklotz
Repo: scalableminds/webknossos PR: 8849
File: frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/collapsible_split_merger_evaluation_settings.tsx:39-51
Timestamp: 2025-08-14T14:12:03.293Z
Learning: Since Ant Design version 5.18, Form.Item components support the layout prop (layout="horizontal", layout="vertical", etc.), allowing individual form items to override the parent Form's layout setting. This is a newer API addition that provides more granular control over form item layouts.
📚 Learning: 2025-08-14T14:12:03.293Z
Learnt from: hotzenklotz
Repo: scalableminds/webknossos PR: 8849
File: frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/collapsible_split_merger_evaluation_settings.tsx:39-51
Timestamp: 2025-08-14T14:12:03.293Z
Learning: Since Ant Design version 5.18, Form.Item components support the layout prop (layout="horizontal", layout="vertical", etc.), allowing individual form items to override the parent Form's layout setting. This is a newer API addition that provides more granular control over form item layouts.
Applied to files:
frontend/javascripts/admin/task/task_create_form_view.tsx
📚 Learning: 2025-08-14T14:12:03.293Z
Learnt from: hotzenklotz
Repo: scalableminds/webknossos PR: 8849
File: frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/collapsible_split_merger_evaluation_settings.tsx:39-51
Timestamp: 2025-08-14T14:12:03.293Z
Learning: Since Ant Design version 5.18, Form.Item components support the layout prop (layout="horizontal", layout="vertical", layout="inline"), allowing individual form items to override the parent Form's layout setting. This enables mixed layouts within a single form and was added as a new API feature in v5.18 according to the official changelog.
Applied to files:
frontend/javascripts/admin/task/task_create_form_view.tsx
🧬 Code graph analysis (1)
frontend/javascripts/admin/task/task_create_form_view.tsx (2)
frontend/javascripts/libs/utils.ts (1)
pluralize(1188-1196)frontend/javascripts/components/async_clickables.tsx (1)
AsyncButton(64-78)
🪛 GitHub Check: frontend-tests
frontend/javascripts/admin/task/task_create_form_view.tsx
[failure] 170-170:
Type '{ showIcon: true; type: "warning"; title: string; description: string; }' is not assignable to type 'IntrinsicAttributes & AlertProps & RefAttributes'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: backend-tests
- GitHub Check: build-smoketest-push
🔇 Additional comments (4)
frontend/javascripts/admin/task/task_create_form_view.tsx (4)
1-1: Imports and utility refactors look consistentSwitching to per-module lodash imports and adding
Alert,Flex,Typography,DownloadOutlined, andpluralizealigns with the new usages below and should be tree-shaking‑friendly. No issues from a typing or bundling perspective.Also applies to: 18-18, 24-24, 34-35, 43-49
176-197: Result modal layout and pluralization logic look solidThe use of
Typography.Paragraph code, copyable failed‑task output,Flexto center the CSV buttons,DownloadOutlinedicons, andpluralizein the modal title all read well and should improve UX. Deduplicating warnings withuniq(warnings)before passing them into the modal is also a nice touch to avoid noisy duplicates.Also applies to: 199-247, 413-416
328-344: UsingomitBy(defaultValues, isNil)forapplyDefaultsis appropriateFiltering out only
null/undefinedvalues before callingform.setFieldsValueis a good way to avoid antd complaining aboutnullfields, while preserving falsy but valid values like0orfalse. The subsequent destructuring to striptypebefore populating the form keeps the form state clean.
518-553:Row align="bottom"changes for form rowsUpdating the dataset, task type, experience, project, and script rows to
align="bottom"(withwrap={false}) should vertically align the right‑hand buttons with the bottom of their associated form items and address the misaligned button issue described in the PR.These rows now have consistent layout semantics, and the change is localized to the admin task form. Just double‑check in the dev instance that this still behaves well on smaller widths where labels wrap.
Also applies to: 604-635, 637-677, 694-725, 727-749
| const warningsContent = | ||
| warnings.length > 0 ? ( | ||
| <div> | ||
| <div style={subHeadingStyle}> | ||
| <WarningOutlined | ||
| style={{ | ||
| color: "var(--ant-color-warning)", | ||
| }} | ||
| />{" "} | ||
| There were warnings during task creation: | ||
| </div> | ||
| <div | ||
| style={{ | ||
| whiteSpace: "pre-line", | ||
| }} | ||
| > | ||
| {warnings.join("\n")} | ||
| </div> | ||
| <Alert | ||
| showIcon | ||
| type="warning" | ||
| title="There were warnings during task creation" | ||
| description={warnings.join("\n")} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "package.json" -type f | head -5Repository: scalableminds/webknossos
Length of output: 139
🏁 Script executed:
rg "antd" package.json | head -20Repository: scalableminds/webknossos
Length of output: 87
🏁 Script executed:
cat frontend/javascripts/admin/task/task_create_form_view.tsx | sed -n '160,180p'Repository: scalableminds/webknossos
Length of output: 627
Replace title with message in Alert component
The Alert component in antd 5.x uses the message prop for the main content, not title. Additionally, render warnings as a list instead of joining with "\n", which won't display as line breaks.
<Alert
showIcon
type="warning"
- title="There were warnings during task creation"
- description={warnings.join("\n")}
+ message="There were warnings during task creation"
+ description={
+ warnings.length === 1 ? (
+ warnings[0]
+ ) : (
+ <ul>
+ {warnings.map((w) => (
+ <li key={w}>{w}</li>
+ ))}
+ </ul>
+ )
+ }
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const warningsContent = | |
| warnings.length > 0 ? ( | |
| <div> | |
| <div style={subHeadingStyle}> | |
| <WarningOutlined | |
| style={{ | |
| color: "var(--ant-color-warning)", | |
| }} | |
| />{" "} | |
| There were warnings during task creation: | |
| </div> | |
| <div | |
| style={{ | |
| whiteSpace: "pre-line", | |
| }} | |
| > | |
| {warnings.join("\n")} | |
| </div> | |
| <Alert | |
| showIcon | |
| type="warning" | |
| title="There were warnings during task creation" | |
| description={warnings.join("\n")} | |
| /> | |
| </div> | |
| const warningsContent = | |
| warnings.length > 0 ? ( | |
| <div> | |
| <Alert | |
| showIcon | |
| type="warning" | |
| message="There were warnings during task creation" | |
| description={ | |
| warnings.length === 1 ? ( | |
| warnings[0] | |
| ) : ( | |
| <ul> | |
| {warnings.map((w) => ( | |
| <li key={w}>{w}</li> | |
| ))} | |
| </ul> | |
| ) | |
| } | |
| /> | |
| </div> |
🧰 Tools
🪛 GitHub Check: frontend-tests
[failure] 170-170:
Type '{ showIcon: true; type: "warning"; title: string; description: string; }' is not assignable to type 'IntrinsicAttributes & AlertProps & RefAttributes'.
🤖 Prompt for AI Agents
In frontend/javascripts/admin/task/task_create_form_view.tsx around lines
164-173, the Alert currently uses the obsolete title prop and joins warnings
with "\n"; replace title with the antd 5.x message prop and render warnings as a
real list instead of a newline-joined string (e.g., set message="There were
warnings during task creation" and set description to a JSX list such as
<ul>{warnings.map((w,i)=><li key={i}>{w}</li>)}</ul> or equivalent) so each
warning displays on its own line.
knollengewaechs
left a comment
knollengewaechs
left a comment
There was a problem hiding this comment.
Thanks for fixing the spacing! The form still works well, but the success modal has a slight linebreak / formatting issue:
Please let me know if I am wrong – I assume this depends on the antd v6 PR, so I will test again after that is merged. Next to that, maybe add a small changelog entry?
|
@knollengewaechs Thanks for the review. I fixed the line break issue and "downgraded" the 
|
knollengewaechs
left a comment
knollengewaechs
left a comment
There was a problem hiding this comment.
nice, thank you!
This PR fixes the alignment of the button in the task creation form. Additionally, it replaces some of the custom CSS with standard antd components for consistency.
URL 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)