adds a placeholder to the voxel size field in upload #8876
Conversation
|
Warning Rate limit exceeded@normanrz has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 37 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
how about preselecting a unit? if 90% of users have the same unit, we could save them a click? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
unreleased_changes/8876.md (1)
1-2: Changelog entry exists — consider expanding scope to reflect all user-visible changesThe PR also tweaks the voxel-size help copy and improves two validation messages (storage quota and unfinished-upload file matching). Adding those here helps downstream users/admins know what to expect.
Apply this diff to expand the entry:
### Added -- Added a placeholder to the voxel size input field in the upload form. +- Upload form: Added a placeholder to the “Voxel Size” input field (e.g., 11.23, 11.23, 28.3). + This clarifies the expected x, y, z format. + +### Changed +- Clarified help text shown for voxel size when a dataset isn’t yet in a WEBKNOSSOS format. +- Improved validation messages: + - Storage quota error now points users/admins to how to request more storage. + - When resuming an unfinished upload, the validator shows which filenames must be selected.frontend/javascripts/admin/dataset/dataset_upload_view.tsx (3)
870-904: Help copy: tighten wording and make intent explicitNit: “a WEBKNOSSOS format” reads a bit awkward. Suggest “WEBKNOSSOS-supported format” and explicitly mention x, y, z to match the field label. Also, since users must provide voxel size when conversion is needed, double-check that the adjacent disabled prop (on the same FormItemWithInfo) isn’t inadvertently disabling the field when needsConversion is true. If it does, please drop it.
Suggested copy tweak:
- help="Your dataset is not yet in a WEBKNOSSOS format. Therefore, you need to define the voxel size." + help="Your dataset is not yet in a WEBKNOSSOS‑supported format. Please define the voxel size (x, y, z) to continue."
992-996: Inline storage-quota validator looks good; consider including numbers for UXLogic is correct and leverages getLeftOverStorageBytes. Optional: include the selected total size and remaining quota in the error to help users decide what to remove. This would require using a custom validator function (to compute a dynamic message) instead of syncValidator’s fixed string.
1020-1034: Tiny copy fix: add a space before the appended filename listCurrently produces “…as before.The file names…”. Add a leading space for readability. Optionally, limit the number of listed files and add “and N more…” to avoid very long messages.
Apply this diff:
- }, "The selected files do not match the files of the unfinished upload. Please select the same files as before." + (unfinishedUploadToContinue?.filePaths != null ? `The file names are: ${unfinishedUploadToContinue?.filePaths?.join(", ")}.` : "")), + }, "The selected files do not match the files of the unfinished upload. Please select the same files as before." + (unfinishedUploadToContinue?.filePaths != null ? ` The file names are: ${unfinishedUploadToContinue?.filePaths?.join(", ")}.` : "")),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
frontend/javascripts/admin/dataset/dataset_upload_view.tsx(4 hunks)unreleased_changes/8876.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/javascripts/admin/dataset/dataset_upload_view.tsx (2)
frontend/javascripts/types/validation.ts (1)
syncValidator(83-86)frontend/javascripts/admin/organization/pricing_plan_utils.ts (1)
getLeftOverStorageBytes(51-53)
⏰ 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: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (1)
frontend/javascripts/admin/dataset/dataset_upload_view.tsx (1)
892-901: Vector3Input uses a single text input; per-axis placeholders aren’t supportedVector3Input (inherited from BaseVector in
frontend/javascripts/libs/vector_input.tsx) renders oneInputComponentthat displays the vector as comma-separated text and forwards the singleplaceholderstring directly to it (see the render method around lines 162–167). It doesn’t render three separate inputs nor accept an array/tuple of placeholders. Keeping<Vector3Input allowDecimals placeholder="e.g. 11.23, 11.23, 28.3" … />is correct.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
frontend/javascripts/admin/dataset/dataset_upload_view.tsx (5)
801-805: Preselected voxel size unit default looks good; aligns with reviewer suggestionDefaulting to UnitLong.nm is a sensible choice and directly addresses fm3’s comment about saving a click for the common case.
If you want to further streamline UX, consider persisting the last used unit in localStorage and using it as the initial value when present (falling back to nm).
872-891: Clarify help text wording and remove an ineffectual disabled prop
- Wording: “in a WEBKNOSSOS format” is ambiguous (WEBKNOSSOS isn’t a file format). Prefer “WEBKNOSSOS‑supported format” for precision.
- The disabled prop on FormItemWithInfo is ts‑ignored and likely ignored at runtime. If the intent is to actually disable the Vector3Input, pass disabled to the input component instead; otherwise, drop the prop to avoid confusion.
Apply this focused copy change and remove the stray prop:
- // @ts-ignore - disabled={this.state.needsConversion} - help="Your dataset is not yet in a WEBKNOSSOS format. Therefore, you need to define the voxel size." + help="Your dataset is not in a WEBKNOSSOS‑supported format. Therefore, you need to define the voxel size."If you do need to disable the control based on state, wire it to Vector3Input:
- <Vector3Input + <Vector3Input + disabled={this.state.needsConversion /* or the intended condition */}
891-900: Nice placeholder; consider reflecting the selected unitThe placeholder is helpful. For even clearer guidance, append the currently selected unit’s short label so users see the expected unit at a glance.
Apply this small tweak:
- <Vector3Input - allowDecimals - placeholder="e.g. 11.23, 11.23, 28.3" + <Vector3Input + allowDecimals + placeholder={`e.g. 11.23, 11.23, 28.3 ${LongUnitToShortUnitMap[ + this.formRef.current?.getFieldValue("voxelSizeUnit") ?? UnitLong.nm + ]}`}
991-995: Storage quota validator: good inline refactor; minor i18n/UX follow-upLogic is correct and efficient. Consider moving the error string into messages for consistency with other i18n’d strings. Also, if organization storage changes while the dialog is open, a quick revalidate on organization updates would keep the error fresh (optional).
1019-1033: Unfinished upload file match: correct set comparison; cap error length and show base namesLogic correctly checks for order-independent equality. However, concatenating all paths can create an extremely long error. Show base names and truncate the list.
Apply this trimming in-place:
- }, "The selected files do not match the files of the unfinished upload. Please select the same files as before." + (unfinishedUploadToContinue?.filePaths != null ? `The file names are: ${unfinishedUploadToContinue?.filePaths?.join(", ")}.` : "")), + }, "The selected files do not match the files of the unfinished upload. Please select the same files as before." + (() => { + const files = unfinishedUploadToContinue?.filePaths; + if (!files) return ""; + const names = files.map((p) => p.split(/[\\/]/).pop() || p); + const shown = names.slice(0, 5).join(", "); + return ` The files are: ${shown}${names.length > 5 ? `, +${names.length - 5} more` : ""}.`; + })()),Optional: if privacy is a concern, keeping only base names (as above) avoids exposing local folder structures in the UI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/javascripts/admin/dataset/dataset_upload_view.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/javascripts/admin/dataset/dataset_upload_view.tsx (2)
frontend/javascripts/types/validation.ts (1)
syncValidator(83-86)frontend/javascripts/admin/organization/pricing_plan_utils.ts (1)
getLeftOverStorageBytes(51-53)
⏰ 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: build-smoketest-push
- GitHub Check: backend-tests
Adds a placeholder to the voxel size field in the upload form with the expected format.
$PR_NUMBER.mdfile inunreleased_changesor use./tools/create-changelog-entry.py)