Skip to content

fix(annotations): list measurements in the order they were placed - #1009

Open
ShivaBollam07 wants to merge 1 commit into
Kitware:mainfrom
ShivaBollam07:fix/list-annotations-in-placement-order
Open

ShivaBollam07 wants to merge 1 commit into
Kitware:mainfrom
ShivaBollam07:fix/list-annotations-in-placement-order

Conversation

@ShivaBollam07

@ShivaBollam07 ShivaBollam07 commented Nov 1, 2026 •

Copy link
Copy Markdown

Fixes #378

Problem

Each view creates its "placing" tool ahead of time, so the order of toolIDs follows when those placeholders were created, not when annotations are finished. With the steps from the issue (ruler in the lower left view, then in the upper left view) the last drawn ruler ends up first in the Measurements list.

Fix

placeTool() moves the tool to the end of toolIDs when it is placed, so finishedTools, the Measurements list and the saved tool order follow placement order. No new field or state file change. Tools stay grouped by type in the list, as before.

@netlify

netlify Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

❌ Deploy Preview for volview-dev failed.

Built without sensitive environment variables

Name Link
🔨 Latest commit 7e532da
🔍 Latest deploy log https://app.netlify.com/projects/volview-dev/deploys/6ae9d2b74bc0d3265edfd376

@PaulHax
PaulHax force-pushed the fix/list-annotations-in-placement-order branch 2 times, most recently from 5673bd1 to 0ca5a56 Compare November 1, 2026 15:14
@PaulHax

PaulHax commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Thank you for taking this on. I prefer, or we're trying to stick with, like a linear history. So never do any merge commits, like what you just did eight hours ago with merging main into this. So do a clean rebase. Make sure that the test quality is high. No need for a testing section inside of the commit, or inside of the PR message. And thank you for your contribution. Of course, can you apply these same principles to your other PR?

@ShivaBollam07

Copy link
Copy Markdown
Author

Thanks for the review, and sorry about the merge commit. I'll rebase this onto main to remove it and keep the history linear, and I'll keep to rebasing from now on.

I've removed the Testing section from the description of this PR and #1010. The commit messages don't have one. I'll also go back over the tests in both PRs, and rebase #1010 as well.

Each view creates its placing tool ahead of time, so tool IDs follow the creation of those
placeholders rather than the order annotations are finished, and a new ruler could show up
in the middle of the Measurements list. Move a tool to the end of the tool list when it is
placed.
@ShivaBollam07
ShivaBollam07 force-pushed the fix/list-annotations-in-placement-order branch from be472b9 to 7e532da Compare November 3, 2026 12:25
@ShivaBollam07

Copy link
Copy Markdown
Author

Rebased this and #1010 onto main, so each is now one commit with no merge commits, and removed the Testing sections. Unit tests and vue-tsc pass locally on both.

The Netlify deploy preview is failing on both PRs, and I don't have access to the deploy log to see the error. Could you take a look when you get a chance?

This branch has not been deployed

No deployments
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.

Annotation order on Measurement List is confusing

2 participants