[measure-tools] Flip condition for measurements in sheets - #1740
Open
Maxime-Brassard wants to merge 5 commits into
Open
[measure-tools] Flip condition for measurements in sheets#1740Maxime-Brassard wants to merge 5 commits into
Maxime-Brassard wants to merge 5 commits into
Conversation
Maxime-Brassard
marked this pull request as ready for review
July 16, 2026 16:16
Maxime-Brassard
requested review from
a team,
a-gagnon and
bsy-nicholasw
as code owners
July 16, 2026 16:16
Maxime-Brassard
enabled auto-merge (squash)
July 16, 2026 16:16
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates sheet-measurement validity logic to be denylist-based instead of allowlist-based, enabling measurements to work when sheet drawing-type metadata is missing while still blocking nonsensical contexts (e.g., Detail drawings).
Changes:
- Replace “allowed drawing types” checks with “forbidden drawing types” checks (tools now forbid
Detailrather than only allowing a small set). - Update
SheetMeasurementHelpertooltip and drawing-type gatekeeping helpers to match the new denylist semantics. - Adjust localized tooltip strings to say “cannot be used on …” and update list-joining wording accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/itwin/measure-tools/src/tools/MeasureLocationTool.ts | Switches sheet validation to use forbiddenDrawingTypes + checkIfNotForbiddenDrawingType. |
| packages/itwin/measure-tools/src/tools/MeasureDistanceTool.ts | Same denylist-based drawing-type gating for distance measurements. |
| packages/itwin/measure-tools/src/tools/MeasureAreaTool.ts | Same denylist-based drawing-type gating for area measurements. |
| packages/itwin/measure-tools/src/api/SheetMeasurementHelper.ts | Introduces checkIfNotForbiddenDrawingType and updates tooltip logic to match denylist semantics (also updates deprecated namespace). |
| packages/itwin/measure-tools/src/api/MeasurementTool.ts | Renames base hook to forbiddenDrawingTypes and wires tooltip generation to it. |
| packages/itwin/measure-tools/public/locales/en/MeasureTools.json | Updates tooltip strings to reflect denylist wording (“cannot be used”). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We flipped the conditions for sheet measurements in order to make them work on models where these details are not provided.
Before, we'd allow using the measurement only inside drawings that were in the allowedDrawingTypes list. Now, instead, we prevent using the measurement inside drawings in the forbiddenDrawingTypes list and allow everywhere else. That way, we're able to use the measurements when the data is not populated and still prevent using the measurements where it does not make sense.