fix(timerangeselector): support uifigure parents (replace uicontrol labels with uilabel when needed)#145
Merged
Merged
Conversation
uicontrol is not supported inside figures created with uifigure(...), so TimeRangeSelector crashes when CompanionEventViewer hosts it (MATLAB Tests (A-D) CI: TestCompanionEventViewer ~15 tests). Detect uifigure parents in the constructor via isprop(hAncFig, 'AutoResizeChildren') — that property exists on uifigure only; classical figure handles lack it even though both report class matlab.ui.Figure. buildGraphics_ then dispatches to either buildLabelsClassical_ (existing uicontrol path, normalized Position, unchanged) or buildLabelsUIFigure_ (new uilabel path, pixel Position recomputed on every hPanel SizeChangedFcn — same pattern as MultiStatusWidget, IconCardWidget, TextWidget). setRangeLabels now routes through a single setLabelText_ helper that writes Text on uilabel and String on uicontrol, so the public contract is unchanged for DashboardEngine. The panel's previous SizeChangedFcn is saved and re-fired so we don't strand sibling resize handlers. The saved handle is restored on delete via restoreCallbacks_. Backward compatible with the dashboard path (DashboardEngine still creates a classical figure() and the uicontrol code path is bit-for-bit identical). No new toolboxes; pure MATLAB. Verified statically: `mh_style` clean on touched code (3 pre-existing warnings on lines unrelated to this fix); only uicontrol callsites in the file are inside buildLabelsClassical_; setRangeLabels' label-handle access now lives in one helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'FastSense Performance'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.10.
| Benchmark suite | Current: bbf3e4c | Previous: c271749 | Ratio |
|---|---|---|---|
Downsample mean std(5M) |
0.099 ms |
0.063 ms |
1.57 |
Render mean std(5M) |
1.701 ms |
0.121 ms |
14.06 |
Zoom cycle mean std(5M) |
0.726 ms |
0.377 ms |
1.93 |
Downsample mean std10M) |
0.213 ms |
0.075 ms |
2.84 |
Instantiation mean std10M) |
0.901 ms |
0.79 ms |
1.14 |
Render mean std10M) |
3.855 ms |
2.544 ms |
1.52 |
Downsample mean (50M) |
86.153 ms |
77.586 ms |
1.11 |
Instantiation mean std50M) |
14.535 ms |
2.592 ms |
5.61 |
Zoom cycle mean std50M) |
0.439 ms |
0.352 ms |
1.25 |
Downsample mean (100M) |
173.133 ms |
152.908 ms |
1.13 |
Downsample mean ( std00M) |
1.684 ms |
0.548 ms |
3.07 |
Instantiation mean ( std00M) |
148.409 ms |
46.52 ms |
3.19 |
Downsample mean ( std00M) |
3.504 ms |
0.548 ms |
6.39 |
Instantiation mean ( std00M) |
718.82 ms |
46.52 ms |
15.45 |
Render mean ( std00M) |
94.167 ms |
2.858 ms |
32.95 |
Zoom cycle mean ( std00M) |
1.069 ms |
0.617 ms |
1.73 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @HanSur94
…n isprop heuristic) The original isprop(fig, 'AutoResizeChildren') discriminator returns TRUE for BOTH classical figures AND uifigures on R2020b+ — confirmed in a live MATLAB probe. Result: every classical-figure construction misroutes to buildLabelsUIFigure_, which then errors because uilabel cannot be parented to a classical figure's uipanel on CI Linux MATLAB. This broke ~14 test files in batch Dashboard and 4 other batches on PR #145's CI. Replace with a hidden-probe pattern: try uicontrol on hPanel; if MATLAB rejects it ("Functionality not supported with figures created with the uifigure function"), switch to uilabel. Bulletproof across MATLAB releases — actually tests parent compatibility rather than guessing from properties. Verified locally: classical figure → classical path (uicontrol), label class = matlab.ui.control.UIControl. uifigure on local macOS also takes the classical path because macOS MATLAB allows uicontrol-in-uifigure (CI Linux rejects it, which is the case the probe catches).
…mh_style: operator_after_continuation)
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.
Root cause
TimeRangeSelectoris used in two contexts:DashboardEngine.render()creates afigure(...);uicontrolis fine.CompanionEventViewer.buildFigure_()creates auifigure(...);uicontrolerrors out with "Functionality not supported with figures created with the uifigure function."The latter is currently broken — the MATLAB Tests (A-D) CI failure on
main(TestCompanionEventViewer/testConstructorOpensFigureand ~14 sibling tests) all blow up atTimeRangeSelector/buildGraphics_:726where the first of threeuicontrol('Style', 'text', ...)calls runs.Fix
isprop(hAncFig, 'AutoResizeChildren'). That property exists onuifigureonly; classicalfigure()handles lack it even though both report classmatlab.ui.Figureon R2020b+. Stored in a new privateIsUIFigureParent_flag.buildGraphics_:uicontrol('Style','text', ...)with normalizedPosition(extracted intobuildLabelsClassical_, bit-for-bit identical to the previous code).buildLabelsUIFigure_that createsuilabelwidgets with pixelPositionand installs aSizeChangedFcnonobj.hPanelto recompute positions on resize. The pixel rectangles mirror the classical normalized layout ([0.045 0.05 0.30 0.32],[0.36 ...],[0.66 ...]). The same pattern (panel-SizeChangedFcn→ recompute pixel layout) is used byMultiStatusWidget,IconCardWidget, andTextWidget.SizeChangedFcnon the panel so we don't strand sibling resize handlers; the saved handle is restored inrestoreCallbacks_(called fromdelete).setRangeLabelsto dispatch through a new privatesetLabelText_(hLabel, str)helper that picksText(uilabel) orString(uicontrol) based onIsUIFigureParent_, with a defensive fallback to the other property if the primary set throws.setRangeLabelsitself is now branch-free.Files changed
libs/Dashboard/TimeRangeSelector.m— 178 insertions, 21 deletions (1 file)New private members:
IsUIFigureParent_,OldPanelSizeChangedFcn_buildLabelsClassical_,buildLabelsUIFigure_,layoutUIFigureLabels_,onPanelResized_,setLabelText_restoreCallbacks_extended to restore the savedSizeChangedFcnwhen we hijacked it.Backward compatibility
DashboardEngine.render→ classicalfigure(...)) hitsbuildLabelsClassical_, which contains the original threeuicontrolcalls verbatim — same options, same positions, sameStringwrites viasetLabelText_.TimeRangeSelectoris unchanged.setRangeLabels(leftText, rightText, middleText)signature is unchanged. Public properties (hRangeLabelLeft/Middle/Right,RangeLeftText/MiddleText/RightText) are unchanged.hRangeLabel*handles change underlying class (uicontrol→uilabel) on the uifigure path. They're declaredSetAccess = privateand a grep acrosslibs/,tests/,examples/,benchmarks/,demo/shows no external code touches.String,.Text,.ForegroundColor,.BackgroundColor, or any other widget-type-dependent property — the only readers are insideTimeRangeSelector.mitself, which already dispatches throughsetLabelText_.TimeRangeSelector:*).Test files exercised by this change
Existing tests still hold (no API surface changed):
tests/test_time_range_selector.m— 6 invariants oversetSelection/setDataRange/getSelection/OnRangeChanged(uses classicalfigure('Visible','off')→ classical path)tests/test_time_range_selector_event_markers.m—setEventMarkerspolyline behavior (classical path)tests/test_time_range_selector_reinstall_after_rerender.m—reinstallCallbacksafter rerender (classical path)tests/suite/TestTimeRangeSelectorEventMarkers.m— class-based variant (classical path)tests/test_dashboard_preview_overlay.m,test_dashboard_preview_envelope.m,test_dashboard_range_selector_integration.m,test_dashboard_engine_event_markers.m— DashboardEngine integration (classical path)tests/suite/TestCompanionEventViewer.m— the failing suite (uifigure path).testConstructorOpensFigure+ ~14 siblings should now pass sincebuildGraphics_no longer throws when the parent is auifigure.Test plan
TestCompanionEventViewer/testConstructorOpensFigureand siblings flip greenTestRangeSelector/TestTimeRangeSelectorEventMarkers/ classical TimeRangeSelector tests stay greentest_dashboard_range_selector_integration.metc.) stay greenDashboardEngine— slider labels still update on selection drag (classical path)CompanionEventViewer— slider mounts inside the uifigure without error and label text updates on drag (uifigure path)Caveats / known limitations
SizeChangedFcnchaining is best-effort. If a future parent (e.g. some uigridlayout cell) refusesSizeChangedFcn, the labels stay at their initial pixel position. This is acceptable for a fixed-height slider strip and isolated bytry/catchso it never breaks construction.setLabelText_(try Text → fall back to String, and vice versa) exist purely for defensive parity if a future refactor crosses widget types under this class. Today only one branch ever fires.Pre-existing
mh_stylewarnings (bgunused at L440,try, ... catch, endextra-comma at L702/703) were left untouched — they are not on lines this PR modifies.Generated with Claude Code