Fix onLoadMore and the info-dialog crash; add a lockable price axis and a gesture-free chart - #2
Merged
Conversation
…toggled onLoadMore was declared and documented but never called: nothing in lib/ invoked it, so paging in older candles could not work. It is now asked at both clamp sites -- the pan handler and the fling listener -- with an edge latch, so a drag pinned against an edge asks once on arrival rather than on every frame, and asks again after coming away and back. The info dialog listened to a single-subscription stream from a subtree that is only mounted while showInfoDialog is set. Toggling the flag true -> false -> true made a second listen on a stream already listened to, which threw 'Bad state: Stream has already been listened to' as the dialog remounted; that throw left the enclosing Stack's children unassigned, surfacing to callers as a LateInitializationError on '_children'. The controller is now a broadcast one, which also keeps delivery asynchronous -- the painter emits from inside paint, so a synchronous notifier would schedule a build during the frame.
ChartStyle.priceAxisWidth reserves a strip on the price axis side -- the side verticalTextAlignment puts the labels on -- and the candles, grid, indicator panes and date axis are all laid out inside what is left. The labels sit in the gutter on their own, so the axis reads cleanly however far the chart is scrolled, rather than candles sliding under the numbers. mWidth is now the width of the plot, with mCanvasWidth for the whole canvas and mPlotLeft/mPlotRight for its edges. The coordinate transforms carry the plot origin, so the crosshair, the drawings, alerts and hit-testing follow the gutter without knowing about it, and the plot is clipped to its own bounds so nothing spills into the axis. Everything that was measured from the canvas edges -- the now-price level and its tag, the crosshair readouts, the high and low callouts, the trading and signal levels, the watermark, the date labels and the pane a press lands in -- is measured from the plot's instead. Pressing the labels grabs the scale, as pressing the axis strip always has. Left at 0, the default, none of this changes what is drawn: the labels stay over the candles and the plot stays the full canvas, which the existing suite and the goldens confirm.
The axis fits the candles in the window, so scrolling rescales it: dragging back through a trend changed every number on it. lockPriceScale holds it at one range instead, and the candles move under a scale that stays put. The lock is applied where the axis scale is worked out rather than where the range is measured, so mMainMaxValue and mMainMinValue stay the honest fit to the window. That keeps the window's high and low pointing at the candles that set them, lets a zoom or a drag work from the locked range, and means the range captured when the lock takes hold is a real fit rather than the last locked value -- without which resetPriceScale could never refit, since it would recapture what it had just cleared. It locks onto the range already on screen, so turning it on does not move the chart. resetPriceScale hands the axis back: it refits to the window and holds there afresh. The range is held until reset, so a chart switching instruments should reset it; paging in candles and live ticks need nothing, which is what the lock is for.
The screenshot tool now takes films as well as stills: a scene whose shutter is called over and over, written out as one looping GIF at 1x, where a still would not show the movement. Bar replay gets one, driven through a transport bar built the way an app would build it. The stills it was missing are here too — the date axis, the legend and crosshair, sizing and theming — and every doc page now opens on the figure it describes.
A locked price axis and a gutter for its labels, and the two reported bugs -- onLoadMore never being called, and the long-press readout throwing when showInfoDialog was set back to true.
scrollEnabled and zoomEnabled turn the chart's own gestures off, for one meant to show a single fixed stretch rather than be navigated -- an intraday session, a thumbnail, a figure in a report. With scrolling off a drag neither slides the window nor flings it, and onLoadMore is never asked, since no edge is reached. With zooming off a pinch does nothing and the web and desktop zoom slider is left off too. The two belong together: zooming out narrows the candles, which leaves the window room to scroll into, so a chart with only scrolling off could be pinched back into a scrollable one. Both hold the user back and leave the controller alone, as priceScaleDrag already did.
A second entry point for the example, showing the other kind of chart: one session drawn as an area, sitting still. scrollEnabled and zoomEnabled off, and pointWidth set to the width over the candle count so the whole session fits. Two switches flip both back on, so the difference can be felt rather than read -- the pinch especially, since zooming out is what would otherwise hand back the scrolling that scrollEnabled took away.
The only thing between the package and 160 pub points: pana runs `dart format --set-exit-if-changed` over the whole package, and this one getter had drifted out of shape -- written against an older formatter, and left behind when it changed how it wraps a cascaded `??=`. Nothing but whitespace and where the lines break.
The benchmark and the render-perf test had drifted out of the formatter's shape. Neither is scored -- pana only weighs lib/ -- but there is no reason for the repository to disagree with `dart format` about anything. The gutter test's paint helper returned a record with an `xs` field that was always empty and never read, left over from an earlier way of checking where things landed. It hands back the counting canvas itself now, which is all any caller wanted.
CtrlAltDevelop
added
bug
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
1 participant
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.
Closes #1
All three items from the report, plus the intraday follow-up. Released as 2.4.0.
Fixed
onLoadMorewas never called. The callback was declared, documented andaccepted, but nothing in
lib/ever invoked it — so paging in older candlescould not work however it was wired up. It is now asked at both places the
scroll is clamped, dragging and flinging, with the edge latched: a drag held
against the edge asks once when it arrives rather than on every frame, and asks
again after coming away and going back.
trueat the newest candle andfalseat the oldest, as documented.
The long-press readout threw when turned off and on again. It listened to a
single-subscription stream from a subtree only mounted while
showInfoDialogisset, so setting it back to
truemade a second listen and threwBad state: Stream has already been listened to. Because that landed while theenclosing
Stackwas mounting its children, callers sawLateInitializationError: Field '_children' has not been initializedinstead —which is what the report pasted. The controller is a broadcast one now.
A
ValueNotifierlooked like the tidier fix, but the painter emits from insidepaint, so notifying synchronously schedules a build during the frame. Thestream's async delivery was load-bearing.
Added
lockPriceScaleholds the price axis at one range instead of refitting it tothe candles in the window. This is what "fixed priceAxis when scroll" turned out
to mean: scrolling rescaled the axis, so dragging back through a trend changed
every number on it.
Locked, the candles move under a scale that stays put. It locks onto what is
already on screen, so turning it on does not move the chart, and
resetPriceScalehands the axis back. The lock is applied where the axis scaleis derived, not where the range is measured, so
mMainMaxValue/mMainMinValuestay the honest window fit — which keeps the high/low markers on the candles that
set them, lets a zoom work off the locked range, and means a reset captures a
real fit rather than the value it just cleared.
ChartStyle.priceAxisWidthholds a gutter back for the labels on whicheverside
verticalTextAlignmentputs them, so candles, grid, panes and the date axisstop short of it instead of sliding under the numbers. The plot is clipped to its
own bounds, and pressing the labels grabs the scale as pressing the axis strip
always has.
scrollEnabledandzoomEnabledturn the chart's own gestures off, for theintraday chart asked about in the follow-up — one session, drawn as an area,
sitting still. Both belong off together: zooming out narrows the candles, which
leaves room to scroll into, so a chart with only scrolling off can be pinched
back into a scrollable one. Both hold the user back and leave
KChartControlleralone, aspriceScaleDragalready did.All four default to off/unchanged, so existing callers are untouched.
Notes
priceAxisWidthis the riskiest change here. Making the plot narrower thanthe canvas meant auditing every "left edge is 0" and "right edge is
mWidth"assumption — 21 sites. One was a real bug I introduced and caught: the
candle-drawing transform did not include the plot origin, so with the axis on
the left everything drew shifted by the gutter width while the crosshair and
drawings used the correct coordinates. Fixed, with a test pinning the transform.
auto-clear-on-instrument-change heuristic: the cheap version misclassifies
paged-in history as a new symbol, and the correct version costs an O(n) scan per
tick, against the grain of the 2.3.1 performance work. Documented instead — call
resetPriceScale()when switching instruments.example/lib/intraday_demo.dartis a second entry point for checking thegesture-free chart by hand:
flutter run -t lib/intraday_demo.dart.a681cff) came from a separate effort onthis branch, not from this one — 6 new images and
example/tool/screenshots.dart.Verification
flutter test— 868 pass, from an 821 baseline; 47 added across four filesflutter analyze— clean, package and exampledart format— 160 files, 0 changedpana— 160/160 pub points (was 150; one unformatted getter inmain_renderer.dartwas costing 10)there, including the exact
Bad state: Stream has already been listened toflutter build ios --simulator— builds, the reporter's platform