Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving the selection of the next active tree after tree deletion #4370

Merged
merged 10 commits into from
Dec 17, 2019
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.md).
- Added a new way of sharing annotations. You can share your annotations with selected teams. These annotations appear in the Shared Annotations Tab in the dashboard. [#4304](https://github.com/scalableminds/webknossos/pull/4304)

### Changed
- Changed the way the new active tree is selected after deleting a tree. Now the tree with the next highest id, compared to the id of the deleted tree, is selected. [#4370](https://github.com/scalableminds/webknossos/pull/4370)
- Consolidates URI handling in the config. Pairs of `uri` and `secured` entries are now specified as just `uri` and require either `http://` or `https://` prefix. [#4368](https://github.com/scalableminds/webknossos/pull/4368)
- Renamed initial organization for the dev deployment to `sample_organization`. [#4368](https://github.com/scalableminds/webknossos/pull/4368)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ class DatasetTable extends React.PureComponent<Props, State> {
/>
<Column
title="Data Layers"
key="dataLayers"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just noticed that react was complaining because the key prop was missing. I just added it here, as an own PR for this change would be overkill.

dataIndex="dataSource.dataLayers"
render={(__, dataset: APIMaybeUnimportedDataset) =>
(dataset.dataSource.dataLayers || []).map(layer => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,20 @@ function getMaximumTreeId(trees: TreeMap): number {
return _.max(_.map(trees, "treeId"));
}

function getNearestTreeId(treeId: number, trees: TreeMap): ?Tree {
const sortedTreeIds = Object.keys(trees)
.map(currentTreeId => parseInt(currentTreeId))
.sort((firstId, secId) => firstId > secId);
if (sortedTreeIds.length === 0) {
return 0;
}
// Uses a binary search to determine the lowest index at which treeId should be inserted into sortedTreeIds in order to maintain its sort order.
// This corresponds to the original index of the deleted treeId.
const originalIndex = _.sortedIndex(sortedTreeIds, treeId);
const higherOrNearestId = Math.min(originalIndex, sortedTreeIds.length - 1);
return sortedTreeIds[higherOrNearestId];
}

export function* mapGroups<R>(
groups: Array<TreeGroup>,
callback: TreeGroup => R,
Expand Down Expand Up @@ -418,8 +432,7 @@ export function deleteBranchPoint(
if (branchPointsAllowed && allowUpdate && hasBranchPoints) {
// Find most recent branchpoint across all trees
const treesWithBranchPoints = _.values(trees).filter(tree => !_.isEmpty(tree.branchPoints));
const treeId = _.maxBy(treesWithBranchPoints, tree => _.last(tree.branchPoints).timestamp)
.treeId;
const { treeId } = _.maxBy(treesWithBranchPoints, tree => _.last(tree.branchPoints).timestamp);
const branchPoint = _.last(trees[treeId].branchPoints);

if (branchPoint) {
Expand Down Expand Up @@ -587,11 +600,10 @@ export function deleteTree(
newActiveTreeId = newTree.treeId;
newActiveNodeId = null;
} else {
// just set the last tree to be the active one
const maxTreeId = getMaximumTreeId(newTrees);
newActiveTreeId = maxTreeId;
// Setting the tree active whose id is the next highest compared to the id of the deleted tree.
newActiveTreeId = getNearestTreeId(tree.treeId, newTrees);
// Object.keys returns strings and the newActiveNodeId should be an integer
newActiveNodeId = +_.first(Array.from(newTrees[maxTreeId].nodes.keys())) || null;
newActiveNodeId = +_.first(Array.from(newTrees[newActiveTreeId].nodes.keys())) || null;
}
const newMaxNodeId = getMaximumNodeId(newTrees);

Expand Down Expand Up @@ -668,7 +680,7 @@ export function createComment(

if (allowUpdate) {
// Gather all comments other than the activeNode's comments
const comments = tree.comments;
const { comments } = tree;
const commentsWithoutActiveNodeComment = comments.filter(comment => comment.nodeId !== node.id);

const newComment: CommentType = {
Expand All @@ -692,7 +704,7 @@ export function deleteComment(
const { allowUpdate } = restrictions;

if (allowUpdate) {
const comments = tree.comments;
const { comments } = tree;
const commentsWithoutActiveNodeComment = comments.filter(comment => comment.nodeId !== node.id);

return Maybe.Just(commentsWithoutActiveNodeComment);
Expand Down