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

Disable proofreading actions for mapped meshes #8091

Merged

Conversation

dieknolle3333
Copy link
Contributor

@dieknolle3333 dieknolle3333 commented Sep 23, 2024

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz (local testing seems better as a special mesh file is needed)

Steps to test:

(see this comment too: #7868 (comment))

  • Select a meshfile that was created for a mapping in proofreading mode (e.g. this one )
  • Load some meshes
  • Open the context menu in the 3D vieport on a mesh
  • If the option "Merge with active segment" is disabled with the tooltip "The mesh wasn't loaded in proofreading mode. Please reload the mesh.", you may need to refresh the meshes
  • Afterwards, if you open the context menu for a mesh in the 3D viewport, the three entries "Merge with active segment", "Split from active segment" and "Split from all neighboring segments" should be disabled with a tooltip that explains that the current mesh file was created for a mapping, but one for the oversegmentation is needed.

Issues:


(Please delete unneeded items, merge only when none are left open)

@dieknolle3333 dieknolle3333 self-assigned this Sep 23, 2024
@dieknolle3333 dieknolle3333 marked this pull request as ready for review September 23, 2024 14:45
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Nice, testing went well 🎉

I only found a very small improvement of your changes. But while looking at the code I found some duplications. It would be very nice if you could refactor some of the duplication away. For more details please see my comments below.

Besides that, this pr should be ready to be :shipit:

frontend/javascripts/oxalis/view/context_menu.tsx Outdated Show resolved Hide resolved
frontend/javascripts/oxalis/view/context_menu.tsx Outdated Show resolved Hide resolved
frontend/javascripts/oxalis/view/context_menu.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Thanks for incorporating my feedback.
Re testing still worked so lets :shipit:

Comment on lines +418 to +428
const getTooltip = (actionVerb: "merge" | "split", actionNeedsActiveSegment: boolean) => {
return !isProofreadingActive
? `Cannot ${actionVerb} because the proofreading tool is not active.`
: maybeUnmappedSegmentId == null
? "The mesh wasn't loaded in proofreading mode. Please reload the mesh."
: meshFileMappingName != null
? "This mesh was created for a mapping. Please use a meshfile that is based on unmapped oversegmentation data."
: actionNeedsActiveSegment && activeSegmentMissing
? "Select a segment first."
: null;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Très bien! 🎉

Comment on lines +430 to +434
const shouldAgglomerateSkeletonActionsBeDisabled =
!isProofreadingActive ||
activeSegmentMissing ||
maybeUnmappedSegmentId == null ||
meshFileMappingName != null;
Copy link
Contributor

Choose a reason for hiding this comment

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

👯

@dieknolle3333 dieknolle3333 enabled auto-merge (squash) September 25, 2024 08:12
@dieknolle3333 dieknolle3333 merged commit 859ab14 into master Sep 25, 2024
2 checks passed
@dieknolle3333 dieknolle3333 deleted the disable-proofreading-actions-for-mapping-meshfiles branch September 25, 2024 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Triggering proofreading actions on precomputed meshes calculated for a mapping should be disabled
3 participants