Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
79b3438
add button to select all trees that match a search
knollengewaechs Oct 14, 2024
e71449d
add function for segments and improve icon
knollengewaechs Oct 16, 2024
7fcca34
remove console log
knollengewaechs Oct 17, 2024
27e841e
Merge branch 'master' into select-all-matching-trees
knollengewaechs Oct 17, 2024
a9ad926
add ts-expect-error tag again
knollengewaechs Oct 17, 2024
526fcc9
Merge branch 'master' into select-all-matching-trees
knollengewaechs Oct 17, 2024
fa56c82
Merge branch 'master' into select-all-matching-trees
knollengewaechs Oct 18, 2024
84301eb
focus first search result and only allow select all matches for leaves
knollengewaechs Oct 18, 2024
9d48473
fix select segment group as search result
knollengewaechs Oct 18, 2024
46c40d9
Merge branch 'master' into select-all-matching-trees
knollengewaechs Oct 21, 2024
c5c32aa
expand parent groups and fix mixed tree and tree group selection
knollengewaechs Oct 21, 2024
b7f484a
changelog
knollengewaechs Oct 21, 2024
304a63a
merge master
knollengewaechs Oct 21, 2024
3c89ebe
lint
knollengewaechs Oct 21, 2024
9d829da
address review
knollengewaechs Oct 24, 2024
0f7c1b7
Merge branch 'master' into select-all-matching-trees
knollengewaechs Oct 24, 2024
a64028b
add placeholder and disable field if all matches all selected
knollengewaechs Oct 25, 2024
13b01f4
Merge branch 'master' into select-all-matching-trees
knollengewaechs Oct 28, 2024
52ef6b4
fix case where group is selected
knollengewaechs Oct 28, 2024
dacdfc3
Merge branch 'master' into select-all-matching-trees
MichaelBuessemeyer Oct 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
- Most sliders have been improved: Wheeling above a slider now changes its value and double-clicking its knob resets it to its default value. [#8095](https://github.com/scalableminds/webknossos/pull/8095)
- It is now possible to search for unnamed segments with the full default name instead of only their id. [#8133](https://github.com/scalableminds/webknossos/pull/8133)
- Increased loading speed for precomputed meshes. [#8110](https://github.com/scalableminds/webknossos/pull/8110)
- Added a button to the search popover in the skeleton and segment tab to select all matching non-group results. [#8123](https://github.com/scalableminds/webknossos/pull/8123)
- Unified wording in UI and code: “Magnification”/“mag” is now used in place of “Resolution“ most of the time, compare [https://docs.webknossos.org/webknossos/terminology.html](terminology document). [#8111](https://github.com/scalableminds/webknossos/pull/8111)

### Changed
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Input, Tooltip, Popover, Space, type InputRef } from "antd";
import { DownOutlined, UpOutlined } from "@ant-design/icons";
import { CheckSquareOutlined, DownOutlined, UpOutlined } from "@ant-design/icons";
import * as React from "react";
import memoizeOne from "memoize-one";
import ButtonComponent from "oxalis/view/components/button_component";
Expand All @@ -11,6 +11,7 @@ type Props<S> = {
data: S[];
searchKey: keyof S | ((item: S) => string);
onSelect: (arg0: S) => void;
onSelectAllMatches?: (arg0: S[]) => void;
children: React.ReactNode;
provideShortcut?: boolean;
targetId: string;
Expand Down Expand Up @@ -109,7 +110,10 @@ export default class AdvancedSearchPopover<
currentPosition =
currentPosition == null ? -1 : Math.min(currentPosition, numberOfAvailableOptions - 1);
const hasNoResults = numberOfAvailableOptions === 0;
const hasMultipleResults = numberOfAvailableOptions > 1;
const availableOptionsToSelectAllMatches = availableOptions.filter(
(result) => result.type === "Tree" || result.type === "segment",
);
const isSelectAllMatchesDisabled = availableOptionsToSelectAllMatches.length < 2;
const additionalInputStyle =
hasNoResults && searchQuery !== ""
? {
Expand Down Expand Up @@ -183,7 +187,7 @@ export default class AdvancedSearchPopover<
width: 40,
}}
onClick={this.selectPreviousOption}
disabled={!hasMultipleResults}
disabled={hasNoResults}
>
<UpOutlined />
</ButtonComponent>
Expand All @@ -194,11 +198,26 @@ export default class AdvancedSearchPopover<
width: 40,
}}
onClick={this.selectNextOption}
disabled={!hasMultipleResults}
disabled={hasNoResults}
>
<DownOutlined />
</ButtonComponent>
</Tooltip>
<Tooltip title="Select all matches (except groups)">
<ButtonComponent
style={{
width: 40,
}}
onClick={
this.props.onSelectAllMatches != null
? () => this.props.onSelectAllMatches!(availableOptionsToSelectAllMatches)
: undefined
}
disabled={isSelectAllMatchesDisabled}
>
<CheckSquareOutlined />
</ButtonComponent>
</Tooltip>
</Space.Compact>
</React.Fragment>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1590,7 +1590,7 @@ class SegmentsView extends React.Component<Props, State> {
this.setState(({ renamingCounter }) => ({ renamingCounter: renamingCounter - 1 }));
};

handleSearchSelect = (selectedElement: SegmentHierarchyNode) => {
maybeExpandParentGroup = (selectedElement: SegmentHierarchyNode) => {
if (this.tree?.current == null) {
return;
}
Expand All @@ -1606,16 +1606,45 @@ class SegmentsView extends React.Component<Props, State> {
if (expandedGroups) {
this.setExpandedGroupsFromSet(expandedGroups);
}
};

handleSearchSelect = (selectedElement: SegmentHierarchyNode) => {
this.maybeExpandParentGroup(selectedElement);
// As parent groups might still need to expand, we need to wait for this to finish.
setTimeout(() => {
if (this.tree.current) this.tree.current.scrollTo({ key: selectedElement.key });
}, 50);
const isASegment = "color" in selectedElement;
if (isASegment) {
this.onSelectSegment(selectedElement);
} else {
if (this.props.visibleSegmentationLayer == null) return;
Store.dispatch(
setSelectedSegmentsOrGroupAction(
[],
selectedElement.id,
this.props.visibleSegmentationLayer?.name,
),
);
}
};

handleSelectAllMatchingSegments = (allMatches: SegmentHierarchyNode[]) => {
if (this.props.visibleSegmentationLayer == null) return;
const allMatchingSegmentIds = allMatches.map((match) => {
this.maybeExpandParentGroup(match);
return match.id;
});
Store.dispatch(
setSelectedSegmentsOrGroupAction(
allMatchingSegmentIds,
null,
this.props.visibleSegmentationLayer.name,
),
);
this.tree.current?.scrollTo({ key: allMatches[0].key });
};

getSegmentStatisticsModal = (groupId: number) => {
const visibleSegmentationLayer = this.props.visibleSegmentationLayer;
if (visibleSegmentationLayer == null) {
Expand Down Expand Up @@ -1833,6 +1862,7 @@ class SegmentsView extends React.Component<Props, State> {
searchKey={(item) => getSegmentName(item)}
provideShortcut
targetId={segmentsTabId}
onSelectAllMatches={this.handleSelectAllMatchingSegments}
>
<ButtonComponent
size="small"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ class SkeletonTabView extends React.PureComponent<Props, State> {
});
};

handleSearchSelect = (selectedElement: TreeOrTreeGroup) => {
maybeExpandParentGroups = (selectedElement: TreeOrTreeGroup) => {
const { skeletonTracing } = this.props;
if (!skeletonTracing) {
return;
Expand All @@ -682,13 +682,25 @@ class SkeletonTabView extends React.PureComponent<Props, State> {
if (expandedGroups) {
this.props.onSetExpandedGroups(expandedGroups);
}
};

handleSearchSelect = (selectedElement: TreeOrTreeGroup) => {
this.maybeExpandParentGroups(selectedElement);
if (selectedElement.type === GroupTypeEnum.TREE) {
this.props.onSetActiveTree(selectedElement.id);
} else {
this.props.onSetActiveTreeGroup(selectedElement.id);
}
};

handleSelectAllMatchingTrees = (matchingTrees: TreeOrTreeGroup[]) => {
const treeIds = matchingTrees.map((tree) => {
this.maybeExpandParentGroups(tree);
return tree.id;
});
this.setState({ selectedTreeIds: treeIds });
};

getTreesComponents(sortBy: string) {
if (!this.props.skeletonTracing) {
return null;
Expand Down Expand Up @@ -864,6 +876,7 @@ class SkeletonTabView extends React.PureComponent<Props, State> {
searchKey="name"
provideShortcut
targetId={treeTabId}
onSelectAllMatches={this.handleSelectAllMatchingTrees}
>
<ButtonComponent title="Open the search via CTRL + Shift + F">
<SearchOutlined />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,7 @@ function TreeHierarchyView(props: Props) {
}
}

function onSelectGroupNode(node: TreeNode) {
const groupId = node.id;
function onSelectGroupNode(groupId: number) {
const numberOfSelectedTrees = props.selectedTreeIds.length;

if (numberOfSelectedTrees > 1) {
Expand Down Expand Up @@ -254,11 +253,11 @@ function TreeHierarchyView(props: Props) {
const checkedKeys = deepFlatFilter(UITreeData, (node) => node.isChecked).map((node) => node.key);

// selectedKeys is mainly used for highlighting, i.e. blueish background color
const selectedKeys = props.selectedTreeIds.map((treeId) =>
getNodeKey(GroupTypeEnum.TREE, treeId),
);
let selectedKeys = props.selectedTreeIds.map((treeId) => getNodeKey(GroupTypeEnum.TREE, treeId));

if (props.activeGroupId) selectedKeys = [getNodeKey(GroupTypeEnum.GROUP, props.activeGroupId)];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why you remove the selected keys here in case there is an active group?

Copy link
Contributor Author

@knollengewaechs knollengewaechs Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes! it is because when selecting multiple elements with ctrl, you cannot select skeletons and skeleton groups at the same time. next to that, there is no multi group selection.
Its not like there is a distiction between active and selected group like for the segments, right?

Copy link
Contributor Author

@knollengewaechs knollengewaechs Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is even this modal which says "You have ${numberOfSelectedTrees} selected Trees. Do you really want to select this group? This will deselect all selected trees."

I thought about adding this here to in this specific case, but it seems hard to realize without very much changing the current architecture:

  • the logic that the selected key is a group is within tree_hierarchy_view (for the skeletons tab)
  • we would need it inAdvancedSearchPopover though, because if the user selects "no" within the modal, we dont want to change the selection. As there is no history to the selection, this needs to happen in the search popover module.
  • the components dont know each other, they are both placed in the SkeletonView

Adding to this, the user pressed the arrow to select the next tree / treegroup, so its hopefully not surprising behaviour. So I personally dont see the need to restructure the code for this to work.
What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I forgot about this limitation.

There is even this modal which says "You have ${numberOfSelectedTrees} selected Trees. Do you really want to select this group? This will deselect all selected trees."

Thanks for the clarification 🙏
So you kinda fixed the code a little here, correct? It behaves "more correctly" than before. I guess the code before was not broken because when a group was selected the code took care of deselecting all trees? Beside that, in regard to your clarification your changes are much nicer than the previous version ✨

So I personally dont see the need to restructure the code for this to work.
What do you think?

Sure, fine by me. If we get the feedback that this should be included in the "search multi selection" then we could still develop this as a follow up :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think I removed inconsistent behaviour regarding the multiselection via ctrl vs the selection in the search. Previously if you had selected skeletons and selected a group via the search, nothing happened to the skeleton selection. I think this wasnt too bad but it doesnt make much sense either. you cannot do any actions with your heterogenous selection anyway, such as drag and drop or actions via a context menu
image


if (props.activeGroupId) selectedKeys.push(getNodeKey(GroupTypeEnum.GROUP, props.activeGroupId));
treeRef.current?.scrollTo({ key: selectedKeys[0], align: "auto" });

return (
<>
Expand Down Expand Up @@ -297,7 +296,7 @@ function TreeHierarchyView(props: Props) {
onSelect={(_selectedKeys, info: { node: TreeNode; nativeEvent: MouseEvent }) =>
info.node.type === GroupTypeEnum.TREE
? onSelectTreeNode(info.node, info.nativeEvent)
: onSelectGroupNode(info.node)
: onSelectGroupNode(info.node.id)
}
onDrop={onDrop}
onCheck={onCheck}
Expand Down