Remove outdated skeleton name input #9154
Conversation
📝 WalkthroughWalkthroughRefactors right-border tab UIs to use Ant Design icons and a simplified ButtonComponent API, migrates several layouts from class-based compact styles to Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
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
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-09-04T10:01:56.727ZApplied to files:
📚 Learning: 2025-12-11T15:25:53.526ZApplied to files:
📚 Learning: 2025-12-11T15:33:06.880ZApplied to files:
📚 Learning: 2025-12-18T13:11:55.113ZApplied to files:
📚 Learning: 2025-12-11T15:54:47.778ZApplied to files:
⏰ 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)
🔇 Additional comments (7)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/stylesheets/trace_view/_right_menu.less (1)
84-90: Complete the Space.Compact migration in comment_tab_view.tsx.The CSS changes at lines 84-90 and 139-149 correctly support the Space.Compact migration. However, the migration is incomplete:
frontend/javascripts/viewer/view/right-border-tabs/comment_tab/comment_tab_view.tsx:456still uses the old className-based pattern (<Space.Compact className="compact-items compact-icons">) without theblockprop. This usage must be updated to align with the migration pattern shown in skeleton_tab_view.tsx.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/javascripts/viewer/view/right-border-tabs/advanced_search_popover.tsx(1 hunks)frontend/javascripts/viewer/view/right-border-tabs/comment_tab/comment_tab_view.tsx(2 hunks)frontend/javascripts/viewer/view/right-border-tabs/connectome_tab/connectome_view.tsx(1 hunks)frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx(4 hunks)frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx(7 hunks)frontend/stylesheets/trace_view/_right_menu.less(12 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-12-11T15:25:53.526Z
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 9117
File: frontend/javascripts/admin/statistic/time_tracking_overview.tsx:261-279
Timestamp: 2025-12-11T15:25:53.526Z
Learning: Ant Design v6 Select: when using the Select component, consider supplying a prefix prop to render an icon or element before the input for better visual context. Apply this guideline to TS and TSX files across the codebase where Ant Design Select is used; ensure prefix usage is accessible (e.g., provide meaningful aria-label if needed) and avoid unnecessary prefixes on simple inputs.
Applied to files:
frontend/javascripts/viewer/view/right-border-tabs/comment_tab/comment_tab_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/advanced_search_popover.tsxfrontend/javascripts/viewer/view/right-border-tabs/connectome_tab/connectome_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx
📚 Learning: 2025-12-11T15:33:06.880Z
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 9117
File: frontend/javascripts/admin/task/task_search_form.tsx:151-151
Timestamp: 2025-12-11T15:33:06.880Z
Learning: In Ant Design v6, do not use optionFilterProp as a standalone prop on Select. Instead, pass it inside showSearch as optionFilterProp, e.g. showSearch={{ optionFilterProp: 'label' }}. showSearch accepts boolean or an object with keys like optionFilterProp, filterOption, autoClearSearchValue. Update all Select components that use optionFilterProp to adopt the new pattern and adjust types accordingly to maintain compatibility and avoid deprecation warnings.
Applied to files:
frontend/javascripts/viewer/view/right-border-tabs/comment_tab/comment_tab_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/advanced_search_popover.tsxfrontend/javascripts/viewer/view/right-border-tabs/connectome_tab/connectome_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx
📚 Learning: 2025-12-11T15:54:47.778Z
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 9117
File: frontend/javascripts/dashboard/advanced_dataset/dataset_table.tsx:131-134
Timestamp: 2025-12-11T15:54:47.778Z
Learning: In Ant Design v6, Dropdown uses a flat classNames structure: classNames={{ root: '…' }}. Other components (Select, AutoComplete, Cascader, TreeSelect) use a nested structure. The deprecated overlayClassName prop for Dropdown should be replaced with classNames.root. In reviews, flag Dropdown usage that relies on overlayClassName and replace it with classNames={{ root: '…' }}. If you encounter related components, verify the correct classNames shape (flat for Dropdown, nested for others) and update accordingly. This guideline covers TSX files under the frontend codebase where Ant Design components are used.
Applied to files:
frontend/javascripts/viewer/view/right-border-tabs/comment_tab/comment_tab_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/advanced_search_popover.tsxfrontend/javascripts/viewer/view/right-border-tabs/connectome_tab/connectome_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_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", 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/stylesheets/trace_view/_right_menu.less
📚 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/stylesheets/trace_view/_right_menu.less
📚 Learning: 2025-12-11T15:25:53.526Z
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 9117
File: frontend/javascripts/admin/statistic/time_tracking_overview.tsx:261-279
Timestamp: 2025-12-11T15:25:53.526Z
Learning: Ant Design v6 Select component supports the `prefix` prop for displaying an icon or element before the selector input.
Applied to files:
frontend/stylesheets/trace_view/_right_menu.less
📚 Learning: 2025-12-08T09:02:43.473Z
Learnt from: hotzenklotz
Repo: scalableminds/webknossos PR: 9117
File: frontend/javascripts/admin/voxelytics/task_list_view.tsx:691-700
Timestamp: 2025-12-08T09:02:43.473Z
Learning: In Ant Design v6, Select (and similar components like AutoComplete, Cascader, TreeSelect) use a nested semantic DOM structure for styles and classNames. The correct pattern is `styles={{ popup: { root: { ... } } }}` or `classNames={{ popup: { root: "..." } }}`, NOT a flat structure. This is the official v6 API for targeting semantic parts of these components.
Applied to files:
frontend/stylesheets/trace_view/_right_menu.lessfrontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx
📚 Learning: 2025-07-15T13:15:37.856Z
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 8689
File: frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx:796-804
Timestamp: 2025-07-15T13:15:37.856Z
Learning: In the WebKnossos codebase, the activeTreeId in skeleton tracing can become null when the active tree is deleted, during initialization, or when trees are deselected via deselectActiveTreeAction. The constructor of SkeletonTabView already handles this null case by setting selectedTreeIds to an empty array.
Applied to files:
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx
🧬 Code graph analysis (2)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (1)
frontend/javascripts/viewer/view/right-border-tabs/advanced_search_popover.tsx (1)
AdvancedSearchPopover(27-229)
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx (1)
frontend/javascripts/viewer/model/actions/annotation_actions.ts (1)
maybeFetchMeshFilesAction(240-254)
⏰ 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 (9)
frontend/javascripts/viewer/view/right-border-tabs/advanced_search_popover.tsx (1)
187-200: LGTM! Consistent UI refactor to ButtonComponent.The changes successfully migrate the search navigation UI to use ButtonComponent throughout, replacing the previous addonAfter status display with a dedicated disabled button. The functionality is preserved while achieving better visual consistency across the right-border tabs.
frontend/stylesheets/trace_view/_right_menu.less (1)
37-62: LGTM! CSS updates support the ButtonComponent migration.The changes to icon visibility (hide by default, show on hover) and hover state styling align with the broader UI refactor that moves to ButtonComponent-based controls. The addition of
.is-hovered-segmentbackground highlighting improves visual feedback.frontend/javascripts/viewer/view/right-border-tabs/connectome_tab/connectome_view.tsx (1)
756-783: LGTM! Space.Compact block prop migration.The addition of the
blockprop to Space.Compact aligns with the broader refactor across right-border-tabs. This ensures consistent block-level rendering without relying on CSS classes.frontend/javascripts/viewer/view/right-border-tabs/comment_tab/comment_tab_view.tsx (1)
508-514: LGTM! Clean migration to Ant Design sort icons.The Sort button now uses the
iconprop with conditional rendering (SortAscendingOutlinedvsSortDescendingOutlined), replacing the previous custom icon rendering logic. This aligns with the broader icon migration to Ant Design across the right-border tabs.frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx (2)
997-1049: LGTM! Comprehensive ButtonComponent migration for mesh controls.The mesh header controls have been successfully migrated to use ButtonComponent wrappers throughout:
- Refresh button uses
icon={<ReloadOutlined />}- Add mesh button wrapped in Popover
- Loading state displays via ButtonComponent with
LoadingOutlined- Settings button wrapped in Popover with
SettingOutlinedThis creates a consistent button-based interaction surface that aligns with the UI standardization across other right-border tabs.
856-871: LGTM! Space.Compact usage for search and mesh controls.The layout now uses Space.Compact to group the search button and mesh header controls, providing consistent spacing and alignment. This matches the pattern used in other right-border tabs (connectome, comment, skeleton).
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (3)
919-974: LGTM! Comprehensive UI migration to Ant Design icons and Space.Compact block.The skeleton tab UI has been successfully updated:
- Space.Compact now uses
blockprop withcompact-wrapclassName- Most buttons migrated to Ant Design icons via
iconprop- Navigation buttons (ArrowLeftOutlined, ArrowRightOutlined) replace previous implementation
- Sort dropdown uses SortAscendingOutlined
- Action buttons (PlusOutlined, DeleteOutlined, SearchOutlined) standardized
Note: Lines 950 and 956 still use FontAwesome icons (
fa-toggle-on,fa-toggle-off) for the toggle visibility buttons. This is acceptable if these specific icons don't have direct Ant Design equivalents or if the migration is intentionally phased.
784-784: LGTM! Consistent icon usage in dropdown and notification.The actions dropdown and notification now use Ant Design icons (SwapOutlined, ArrowsAltOutlined) consistently with the button bar updates. This ensures visual consistency across all UI surfaces in the skeleton tab.
Also applies to: 832-832, 847-847
34-38: Remove incorrect claim about export removal.The imports no longer include
getActiveTreeandgetActiveTreeGroupfrom skeletontracing_accessor—verified these functions are not used in skeleton_tab_view.tsx. However, these functions are still exported from skeletontracing_accessor.ts (lines 82, 95), so the statement that they were "removed from public exports" is incorrect. The import removal is appropriate for this file's refactoring.
| import React from "react"; | ||
|
|
||
| type ButtonComponentProps = ButtonProps & { | ||
| faIcon?: string; |
There was a problem hiding this comment.
I did not find any usages of the faIcon prop
| function renderSortIcon() { | ||
| const sortAsc = isSortedAscending; | ||
| const sortNumeric = sortBy === SortByEnum.ID; | ||
| const iconClass = `fas fa-sort-${sortNumeric ? "numeric" : "alpha"}-${sortAsc ? "down" : "up"}`; |
There was a problem hiding this comment.
While this detailed icon switch is nice, I feel like it does not add much value. Antd does not have separate icons for sorting text and numbers so I simplified it to just asc/desc sorting.
| </Popover> | ||
| </FastTooltip> | ||
| <ButtonComponent | ||
| title="Refresh list of available Mesh files" |
There was a problem hiding this comment.
A <ButtonComponent> is already wrapped with a FastTooltip if using the title prop.
philippotto
left a comment
philippotto
left a comment
There was a problem hiding this comment.
overall looks good 👍 thanks for doing this. I noticed one problem during testing (however, it is already a bug on the master apparently):
when adding a comment to a skeleton node and opening the markdown modal, the text box in the modal is empty. in general, the modal text box isn't properly synced.
if you feel like it, have a look whether you can fix it in this PR. however, it's not blocking this PR.
I had a look into this and fixed the issue as well. see commit |
|
excellent |
This PR does a few things to the skeleton, comment and segments view:
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)