[Components]: apply constraints for quantity editor#1323
Conversation
🦋 Changeset detectedLatest commit: 4f87ba4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
Extends the property constraint enforcement work from #834 to the quantity editor path, so ECProperty minimumValue/maximumValue constraints are also applied when editing KOQ-formatted values in the property grid.
Changes:
- Added shared helpers to extract min/max constraints and clamp numeric values.
- Updated quantity editor commit flow to clamp persisted (raw) values to constraints and update formatted display accordingly.
- Added/expanded Vitest coverage for constraint behavior (including persistence/display unit conversion), plus a small test-app wiring tweak.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/test/properties/inputs/Utils.test.ts | Adds unit tests for new shared constraint helpers. |
| packages/components/src/test/properties/inputs/QuantityPropertyEditorInput.test.tsx | Adds coverage ensuring quantity edits are clamped to min/max (including unit conversion scenarios). |
| packages/components/src/presentation-components/properties/inputs/Utils.ts | Introduces shared getMinMaxFromPropertyConstraints and applyNumericConstraints helpers. |
| packages/components/src/presentation-components/properties/inputs/UseQuantityValueInput.tsx | Adds setNewValue helper to update state/formatting when clamping on commit. |
| packages/components/src/presentation-components/properties/inputs/QuantityPropertyEditorInput.tsx | Applies min/max constraints on blur before committing the new value. |
| packages/components/src/presentation-components/properties/inputs/NumericPropertyInput.tsx | Refactors numeric constraint application to reuse shared helpers. |
| apps/test-app/frontend/src/components/properties-widget/PropertiesWidget.tsx | Wraps the grid with IModelConnectionProvider (test-app wiring for the new editor system). |
…uts/Utils.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Unified selection benchmark
| Benchmark suite | Current: 5192bb1 | Previous: cffc069 | Deviation | Status |
|---|---|---|---|---|
compute selection for 50k elements |
363 ms |
344 ms |
5.52% |
〰️ |
compute selection for 50k elements (P95 of main thread blocks) |
31 ms |
31 ms |
0% |
🟰 |
compute parent selection for 50k elements |
342 ms |
346 ms |
-1.16% |
〰️ |
compute parent selection for 50k elements (P95 of main thread blocks) |
31 ms |
32 ms |
-3.13% |
〰️ |
compute top ancestor selection for 50k elements |
580 ms |
553 ms |
4.88% |
〰️ |
compute top ancestor selection for 50k elements (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
compute category selection for 50k elements |
103 ms |
104 ms |
-0.96% |
〰️ |
compute category selection for 50k elements (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
compute model selection for 50k elements |
73 ms |
74 ms |
-1.35% |
〰️ |
compute model selection for 50k elements (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
compute functional selection for 50k 3D elements |
404 ms |
410 ms |
-1.46% |
〰️ |
compute functional selection for 50k 3D elements (P95 of main thread blocks) |
31 ms |
31 ms |
0% |
🟰 |
compute parent functional selection for 50k 3D elements |
430 ms |
434 ms |
-0.92% |
〰️ |
compute parent functional selection for 50k 3D elements (P95 of main thread blocks) |
31 ms |
31 ms |
0% |
🟰 |
compute top ancestor functional selection for 50k 3D elements |
1162 ms |
1163 ms |
-0.09% |
〰️ |
compute top ancestor functional selection for 50k 3D elements (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
compute functional selection for 50k 2D elements |
2942 ms |
2926 ms |
0.55% |
〰️ |
compute functional selection for 50k 2D elements (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
compute parent functional selection for 50k 2D elements |
2891 ms |
2835 ms |
1.98% |
〰️ |
compute parent functional selection for 50k 2D elements (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
compute top ancestor functional selection for 50k 2D elements |
2978 ms |
2922 ms |
1.92% |
〰️ |
compute top ancestor functional selection for 50k 2D elements (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
hilite 50k elements |
1235 ms |
1160 ms |
6.47% |
〰️ |
hilite 50k elements (P95 of main thread blocks) |
58 ms |
51 ms |
13.73% |
〰️ |
hilite 50k group elements |
242 ms |
238 ms |
1.68% |
〰️ |
hilite 50k group elements (P95 of main thread blocks) |
35 ms |
35 ms |
0% |
🟰 |
hilite 1k subjects |
48313 ms |
48071 ms |
0.50% |
〰️ |
hilite 1k subjects (P95 of main thread blocks) |
24 ms |
28 ms |
-14.29% |
〰️ |
hilite 50k subcategories |
298 ms |
315 ms |
-5.40% |
〰️ |
hilite 50k subcategories (P95 of main thread blocks) |
39 ms |
40 ms |
-2.50% |
〰️ |
hilite 50k functional 3D elements |
27133 ms |
26812 ms |
1.20% |
〰️ |
hilite 50k functional 3D elements (P95 of main thread blocks) |
40 ms |
42 ms |
-4.76% |
〰️ |
hilite 50k functional 2D elements |
6085 ms |
6000 ms |
1.42% |
〰️ |
hilite 50k functional 2D elements (P95 of main thread blocks) |
35 ms |
32 ms |
9.38% |
〰️ |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Hierarchies benchmark
| Benchmark suite | Current: 5192bb1 | Previous: cffc069 | Deviation | Status |
|---|---|---|---|---|
filtering filters with 50000 paths |
8270 ms |
6536 ms |
26.53% |
🚨 |
filtering filters with 50000 paths (P95 of main thread blocks) |
234 ms |
182 ms |
28.57% |
🚨 |
hide if no children required to finalize root, w/o children |
40028 ms |
28058 ms |
42.66% |
🚨 |
hide if no children required to finalize root, w/o children (P95 of main thread blocks) |
42 ms |
25 ms |
68% |
〰️ |
hide if no children required to finalize root, w/ children |
181 ms |
135 ms |
34.07% |
〰️ |
hide if no children required to finalize root, w/ children (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
models tree initial (Baytown) |
94 ms |
63 ms |
49.21% |
〰️ |
models tree initial (Baytown) (P95 of main thread blocks) |
27 ms |
0 ms |
2700% |
〰️ |
models tree full (Baytown) |
6377 ms |
5708 ms |
11.72% |
🚨 |
models tree full (Baytown) (P95 of main thread blocks) |
80 ms |
67 ms |
19.40% |
〰️ |
models tree creates initial filtered view for 50k target items |
2226 ms |
1695 ms |
31.33% |
🚨 |
models tree creates initial filtered view for 50k target items (P95 of main thread blocks) |
55 ms |
34 ms |
61.76% |
〰️ |
grouping by label |
14038 ms |
11564 ms |
21.39% |
🚨 |
grouping by label (P95 of main thread blocks) |
59 ms |
54 ms |
9.26% |
〰️ |
grouping by class |
13854 ms |
11439 ms |
21.11% |
🚨 |
grouping by class (P95 of main thread blocks) |
51 ms |
42 ms |
21.43% |
〰️ |
grouping by property |
14517 ms |
12263 ms |
18.38% |
🚨 |
grouping by property (P95 of main thread blocks) |
66 ms |
57 ms |
15.79% |
〰️ |
grouping by base class (10 classes) |
12259 ms |
10349 ms |
18.46% |
🚨 |
grouping by base class (10 classes) (P95 of main thread blocks) |
103 ms |
99 ms |
4.04% |
〰️ |
grouping by multiple attributes |
29701 ms |
24710 ms |
20.20% |
🚨 |
grouping by multiple attributes (P95 of main thread blocks) |
52 ms |
75 ms |
-30.67% |
〰️ |
flat 50k elements list |
9057 ms |
3564 ms |
154.12% |
🚨 |
flat 50k elements list (P95 of main thread blocks) |
106 ms |
51 ms |
107.84% |
🚨 |
This comment was automatically generated by workflow using github-action-benchmark.
Co-authored-by: Copilot <copilot@github.com>
Part of #779.
Previous changes made in #834 applied only for numeric editors. Adjusted quantity editor to also support this.
To complete #779, changes in iTwin/appui#1544 should be merged and then consumed in https://github.com/iTwin/viewer-components-react/tree/master/packages/itwin/property-grid