fix(Progress): clear stale segment values when flipping to indeterminate - #843
Open
sridhar-3009 wants to merge 3 commits into
Open
fix(Progress): clear stale segment values when flipping to indeterminate#843sridhar-3009 wants to merge 3 commits into
sridhar-3009 wants to merge 3 commits into
Conversation
apply() only wrote incoming values to segments at matching array indices, so apply([]) - what happens when a bound model value becomes undefined - left previously-registered segments at their old value. Flipping EmProgress's indeterminate prop after a value was already committed ran the CSS sweep animation correctly but data-state stayed "determinate" and aria-valuenow/aria-valuetext kept reporting the stale committed value to assistive tech. apply() now walks every registered segment instead of just the incoming array, resetting any segment without a corresponding entry back to min. Also fixes a stale comment in EmProgress.vue claiming ProgressRoot always emits aria-labelledby; it's gated on hasLabel.
…set branches Adds two apply() cases the codecov patch check flagged as uncovered: skipping a readonly segment, and resetting a segment with no corresponding incoming entry back to min.
…ances The initial value option pinned isIndeterminate to false permanently, even after segments registered and were cleared back to min. Scope the pin to the zero-segment state, mirroring total's fallback, so registered segments are the sole source of truth once they exist.
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.
Fixes #814
Root cause
EmProgress.vuepasses:model-value="indeterminate ? undefined : model"toProgress.Root. Whenindeterminateflips totrueafter a value was already committed,ProgressRoot'sinternalcomputed collapses to[], anduseProxyModelcallscontext.apply([]).createProgress.apply()only wrote incoming values to segments at matching array indices:clamped.entries()is empty whenincomingis[], so the loop body never runs — any already-registered segment keeps its old value.isIndeterminatederives from segment values (> 0means determinate), so it stayedfalse, andProgressRootkept emittingdata-state="determinate"and the stalearia-valuenow/aria-valuetexteven while Emerald's CSS sweep animation was visibly running — a real accessibility bug (AT reports a fixed value while the bar visually reads as busy/indeterminate).Fix
apply()now iterates over every registered segment instead of only the incoming array, so a segment with no corresponding entry resets tomin:Since
apply()is always called with the full desired model state (seeuseProxyModel'sreconcile), this is the correct general semantic — not specific to the indeterminate case — and doesn't affect any of the existingapplytests (single segment, multi-segment, clamping, pending-registration).Also fixed the smaller item the issue flagged: the stale comment in
EmProgress.vueclaimingProgressRootalways emitsaria-labelledby— it's gated onhasLabel(ProgressRoot.vue:144).Test plan
v-modeltest that commits a value, then transitions toundefinedviawrapper.setProps— failed onmain(totalstayed60instead of resetting to0), passes with the fixpnpm vitest run --project v0:unit→ 4700 passed, 1 skipped (no change from baseline)pnpm --filter=@vuetify/v0 typecheckandpnpm --filter=@paper/emerald typecheckcleanpnpm eslintclean on changed files@vuetify/v0and@paper/emerald)