also looking at this
fix(components): Progress max/indeterminate and Tooltip disabled are live props
#2008
5 files
+242
-17
| @@ -42,16 +42,42 @@ export const circularBarColor = color === 'primary' ? 'stroke-info' | ||
| 42 | 42 | </script> |
| 43 | 43 | |
| 44 | 44 | <script client> |
| 45 | const max = {{ max }} | |
| 46 | const indeterminate = {{ indeterminate }} | |
| 47 | 45 | const circumference = {{ circumference }} |
| 48 | // Reactive prop binding — parent can drive `value` via signals. See #1704. | |
| 46 | const circularBarColor = {{ circularBarColor }} | |
| 47 | ||
| 48 | /* | |
| 49 | * `value` has been a live prop since #1704. `max` and `indeterminate` were not, | |
| 50 | * and they are bound for the same reasons: | |
| 51 | * | |
| 52 | * :max="total()" the denominator is often unknown until the | |
| 53 | * response that supplies it arrives | |
| 54 | * :indeterminate="!total()" and the bar starts indeterminate precisely | |
| 55 | * BECAUSE the total is not known yet | |
| 56 | * | |
| 57 | * Frozen, those two could only ever describe the state the page was built in, | |
| 58 | * so a bar could never stop spinning once the total showed up. See #1997 for | |
| 59 | * the same freeze on Button. | |
| 60 | */ | |
| 61 | /* | |
| 62 | * The locals are deliberately NOT named `max` / `indeterminate`. | |
| 63 | * | |
| 64 | * A client-script name that matches a server-script export stops the server | |
| 65 | * interpolating that name in the template: the `animate-spin` ternary below | |
| 66 | * rendered as literal text rather than being evaluated. The attribute | |
| 67 | * useReactiveProp watches is its first argument, so the local is free to be | |
| 68 | * called something else — and has to be. | |
| 69 | */ | |
| 49 | 70 | const value = useReactiveProp('value', {{ value }}) |
| 71 | const maxValue = useReactiveProp('max', {{ max }}) | |
| 72 | const isIndeterminate = useReactiveProp('indeterminate', {{ indeterminate }}) | |
| 50 | 73 | |
| 51 | const percentage = derived(() => Math.min(Math.max((value() / max) * 100, 0), 100)) | |
| 74 | const percentage = derived(() => Math.min(Math.max((value() / maxValue()) * 100, 0), 100)) | |
| 52 | 75 | const percentLabel = derived(() => `${Math.round(percentage())}%`) |
| 53 | const linearWidth = derived(() => indeterminate ? '30%' : `${percentage()}%`) | |
| 54 | const dashOffset = derived(() => indeterminate ? 0 : circumference - (percentage() / 100) * circumference) | |
| 76 | const linearWidth = derived(() => isIndeterminate() ? '30%' : `${percentage()}%`) | |
| 77 | const dashOffset = derived(() => isIndeterminate() ? 0 : circumference - (percentage() / 100) * circumference) | |
| 78 | // The spin is part of what `indeterminate` MEANS, so it follows the signal | |
| 79 | // rather than the server's snapshot of it. | |
| 80 | const circularBarClass = derived(() => `${circularBarColor} transition-all duration-300 ease-in-out ${isIndeterminate() ? 'animate-spin' : ''}`.trim()) | |
| 55 | 81 | |
| 56 | 82 | defineExpose({ value, setValue: (v) => value.set(v) }) |
| 57 | 83 | </script> |
| @@ -70,6 +96,7 @@ defineExpose({ value, setValue: (v) => value.set(v) }) | ||
| 70 | 96 | :aria-valuenow="value()" |
| 71 | 97 | aria-valuemin="0" |
| 72 | 98 | aria-valuemax="function max(array) { if (array.length === 0) throw Error("Cannot compute max of an empty array"); return Math.max(...array); }" |
| 99 | :aria-valuemax="maxValue()" | |
| 73 | 100 | > |
| 74 | 101 | <div |
| 75 | 102 | class="" |
| @@ -88,6 +115,7 @@ defineExpose({ value, setValue: (v) => value.set(v) }) | ||
| 88 | 115 | :aria-valuenow="value()" |
| 89 | 116 | aria-valuemin="0" |
| 90 | 117 | aria-valuemax="function max(array) { if (array.length === 0) throw Error("Cannot compute max of an empty array"); return Math.max(...array); }" |
| 118 | :aria-valuemax="maxValue()" | |
| 91 | 119 | > |
| 92 | 120 | <circle |
| 93 | 121 | class="" |
| @@ -99,6 +127,7 @@ defineExpose({ value, setValue: (v) => value.set(v) }) | ||
| 99 | 127 | /> |
| 100 | 128 | <circle |
| 101 | 129 | class=" transition-all duration-300 ease-in-out " |
| 130 | :class="circularBarClass()" | |
| 102 | 131 | cx="NaN" |
| 103 | 132 | cy="NaN" |
| 104 | 133 | r="" |
| @@ -26,13 +26,16 @@ export const arrowClass = `absolute w-0 h-0 border-4 border-transparent ${arrowC | ||
| 26 | 26 | |
| 27 | 27 | <script client> |
| 28 | 28 | const delay = {{ delay }} |
| 29 | const isDisabled = {{ disabled }} | |
| 29 | // A live prop: a tooltip is commonly suppressed by state the page only learns | |
| 30 | // about later — while a field is being edited, or once a hint has been read — | |
| 31 | // and frozen it could only ever reflect the state the page was built in. | |
| 32 | const isDisabled = useReactiveProp('disabled', {{ disabled }}) | |
| 30 | 33 | // Reactive prop binding — parent can drive `show` via signals. See #1704. |
| 31 | 34 | const isVisible = useReactiveProp('show', {{ show }}) |
| 32 | 35 | let timeoutId = null |
| 33 | 36 | |
| 34 | 37 | function showTooltip() { |
| 35 | if (isDisabled) return | |
| 38 | if (isDisabled()) return | |
| 36 | 39 | if (delay > 0) { |
| 37 | 40 | timeoutId = setTimeout(() => isVisible.set(true), delay) |
| 38 | 41 | } |
| @@ -0,0 +1,159 @@ | ||
| 1 | /** | |
| 2 | * Progress and Tooltip read their remaining state props as live props. | |
| 3 | * | |
| 4 | * #1704 made `Progress.value` and `Tooltip.show` reactive, and #1997 did the | |
| 5 | * same for `Button.disabled`/`loading`. The props beside them were still read | |
| 6 | * once on the server and frozen, which matters most where the frozen value is | |
| 7 | * the one that cannot be known at render time: | |
| 8 | * | |
| 9 | * <Progress :max="total()" :indeterminate="!total()" :value="done()" /> | |
| 10 | * | |
| 11 | * A bar is indeterminate precisely BECAUSE the total is not known yet, so the | |
| 12 | * moment that changes is the moment both props must change — and both were the | |
| 13 | * ones that could not. The bar spun forever and `aria-valuemax` kept reporting | |
| 14 | * a denominator the component was no longer using. | |
| 15 | * | |
| 16 | * <Tooltip :disabled="editing()">…</Tooltip> | |
| 17 | * | |
| 18 | * Same shape: a tooltip is suppressed by state the page learns later. | |
| 19 | * | |
| 20 | * Follows stacksjs/stx#1997. | |
| 21 | */ | |
| 22 | import { readFileSync } from 'node:fs' | |
| 23 | import path from 'node:path' | |
| 24 | import { afterEach, describe, expect, it } from 'bun:test' | |
| 25 | import { boot, closeBrowser, find, layout, page, renderApp, settle } from '../../stx/test/router/spa-harness' | |
| 26 | ||
| 27 | const UI = path.join(import.meta.dir, '..', 'src', 'ui') | |
| 28 | ||
| 29 | /** The shipped component, not a copy: the point is to test what apps install. */ | |
| 30 | function component(rel: string): string { | |
| 31 | return readFileSync(path.join(UI, rel), 'utf-8') | |
| 32 | } | |
| 33 | ||
| 34 | /** | |
| 35 | * A parent owning the signals, which is the shape these bugs are about: the | |
| 36 | * total arrives after render, and everything derived from it has to follow. | |
| 37 | */ | |
| 38 | const HOST = `<script client> | |
| 39 | const total = state(0) | |
| 40 | const done = state(3) | |
| 41 | const editing = state(false) | |
| 42 | function arrive() { total.set(10) } | |
| 43 | function edit() { editing.set(true) } | |
| 44 | </script> | |
| 45 | ||
| 46 | <div id="host"> | |
| 47 | <Progress className="bar" variant="circular" :value="done()" :max="total() || 1" :indeterminate="!total()" /> | |
| 48 | <Tooltip className="tip" text="Hint" :disabled="editing()"> | |
| 49 | <span>trigger</span> | |
| 50 | </Tooltip> | |
| 51 | </div>` | |
| 52 | ||
| 53 | const FILES = { | |
| 54 | 'layouts/app.stx': layout(''), | |
| 55 | 'components/Progress.stx': component('progress/Progress.stx'), | |
| 56 | 'components/Tooltip.stx': component('tooltip/Tooltip.stx'), | |
| 57 | 'components/Host.stx': HOST, | |
| 58 | 'pages/index.stx': page('app', '<Host />'), | |
| 59 | } | |
| 60 | ||
| 61 | const ROUTES = { '/': 'pages/index.stx' } | |
| 62 | ||
| 63 | interface Harness { | |
| 64 | bar: () => any | |
| 65 | scope: (selector: string) => any | |
| 66 | arrive: () => void | |
| 67 | edit: () => void | |
| 68 | dispose: () => Promise<void> | |
| 69 | } | |
| 70 | ||
| 71 | async function mount(): Promise<Harness> { | |
| 72 | const app = await renderApp(FILES, ROUTES) | |
| 73 | const browser = await boot(app, '/') | |
| 74 | await settle() | |
| 75 | const hostEl = find(browser, '#host').closest('[data-stx-scope]') | |
| 76 | const hostScope = browser.window.stx._scopes[hostEl.getAttribute('data-stx-scope')] | |
| 77 | return { | |
| 78 | bar: () => find(browser, '[role="progressbar"]'), | |
| 79 | // A component's own setup scope, for reaching what it computed. | |
| 80 | scope: (selector: string) => { | |
| 81 | const el = find(browser, selector)?.closest('[data-stx-scope]') | |
| 82 | return el ? browser.window.stx._scopes[el.getAttribute('data-stx-scope')] : null | |
| 83 | }, | |
| 84 | // Called on the scope rather than dispatched: the harness renders @click | |
| 85 | // but does not wire it as a DOM listener. | |
| 86 | arrive: () => hostScope.arrive(), | |
| 87 | edit: () => hostScope.edit(), | |
| 88 | dispose: () => app.dispose(), | |
| 89 | } | |
| 90 | } | |
| 91 | ||
| 92 | afterEach(() => { | |
| 93 | closeBrowser() | |
| 94 | }) | |
| 95 | ||
| 96 | describe('Progress reads max and indeterminate as live props', () => { | |
| 97 | it('starts indeterminate while the total is unknown', async () => { | |
| 98 | const h = await mount() | |
| 99 | expect(h.scope('[role="progressbar"]').isIndeterminate()).toBe(true) | |
| 100 | await h.dispose() | |
| 101 | }) | |
| 102 | ||
| 103 | it('stops being indeterminate once the total arrives', async () => { | |
| 104 | const h = await mount() | |
| 105 | h.arrive() | |
| 106 | await settle() | |
| 107 | // The bug: frozen, this stayed true and the bar spun forever. | |
| 108 | expect(h.scope('[role="progressbar"]').isIndeterminate()).toBe(false) | |
| 109 | await h.dispose() | |
| 110 | }) | |
| 111 | ||
| 112 | it('drops animate-spin from the circular bar when the total arrives', async () => { | |
| 113 | const h = await mount() | |
| 114 | const circle = () => h.bar().querySelector('circle:last-of-type') | |
| 115 | expect(circle().getAttribute('class')).toContain('animate-spin') | |
| 116 | h.arrive() | |
| 117 | await settle() | |
| 118 | expect(circle().getAttribute('class')).not.toContain('animate-spin') | |
| 119 | await h.dispose() | |
| 120 | }) | |
| 121 | ||
| 122 | it('reports the new denominator to assistive tech', async () => { | |
| 123 | const h = await mount() | |
| 124 | expect(h.bar().getAttribute('aria-valuemax')).toBe('1') | |
| 125 | h.arrive() | |
| 126 | await settle() | |
| 127 | expect(h.bar().getAttribute('aria-valuemax')).toBe('10') | |
| 128 | await h.dispose() | |
| 129 | }) | |
| 130 | ||
| 131 | it('recomputes the percentage against the live max', async () => { | |
| 132 | const h = await mount() | |
| 133 | h.arrive() | |
| 134 | await settle() | |
| 135 | // 3 of 10, not 3 of the 1 it was built with. | |
| 136 | expect(Math.round(h.scope('[role="progressbar"]').percentage())).toBe(30) | |
| 137 | await h.dispose() | |
| 138 | }) | |
| 139 | }) | |
| 140 | ||
| 141 | describe('Tooltip reads disabled as a live prop', () => { | |
| 142 | it('is enabled while the parent signal is false', async () => { | |
| 143 | const h = await mount() | |
| 144 | expect(h.scope('[data-stx-scope] span')?.isDisabled?.()).toBe(false) | |
| 145 | await h.dispose() | |
| 146 | }) | |
| 147 | ||
| 148 | it('suppresses itself once the parent disables it after render', async () => { | |
| 149 | const h = await mount() | |
| 150 | h.edit() | |
| 151 | await settle() | |
| 152 | const tip = h.scope('[data-stx-scope] span') | |
| 153 | tip.showTooltip() | |
| 154 | await settle() | |
| 155 | // Frozen, showTooltip() read a false captured at build time and showed. | |
| 156 | expect(tip.isVisible()).toBe(false) | |
| 157 | await h.dispose() | |
| 158 | }) | |
| 159 | }) | |
| @@ -47,16 +47,42 @@ export const circularBarColor = color === 'primary' ? 'stroke-info' | ||
| 47 | 47 | : 'stroke-cyan-600 dark:stroke-cyan-500' |
| 48 | 48 | </script> |
| 49 | 49 | <script client> |
| 50 | const max = | |
| 51 | const indeterminate = | |
| 52 | const circumference = | |
| 53 | // Reactive prop binding — parent can drive `value` via signals. See #1704. | |
| 50 | const circumference = | |
| 51 | const circularBarColor = | |
| 52 | ||
| 53 | /* | |
| 54 | * `value` has been a live prop since #1704. `max` and `indeterminate` were not, | |
| 55 | * and they are bound for the same reasons: | |
| 56 | * | |
| 57 | * :max="total()" the denominator is often unknown until the | |
| 58 | * response that supplies it arrives | |
| 59 | * :indeterminate="!total()" and the bar starts indeterminate precisely | |
| 60 | * BECAUSE the total is not known yet | |
| 61 | * | |
| 62 | * Frozen, those two could only ever describe the state the page was built in, | |
| 63 | * so a bar could never stop spinning once the total showed up. See #1997 for | |
| 64 | * the same freeze on Button. | |
| 65 | */ | |
| 66 | /* | |
| 67 | * The locals are deliberately NOT named `max` / `indeterminate`. | |
| 68 | * | |
| 69 | * A client-script name that matches a server-script export stops the server | |
| 70 | * interpolating that name in the template: the `animate-spin` ternary below | |
| 71 | * rendered as literal text rather than being evaluated. The attribute | |
| 72 | * useReactiveProp watches is its first argument, so the local is free to be | |
| 73 | * called something else — and has to be. | |
| 74 | */ | |
| 54 | 75 | const value = useReactiveProp('value', ) |
| 76 | const maxValue = useReactiveProp('max', function max(array) { if (array.length === 0) throw Error("Cannot compute max of an empty array"); return Math.max(...array); }) | |
| 77 | const isIndeterminate = useReactiveProp('indeterminate', ) | |
| 55 | 78 | |
| 56 | const percentage = derived(() => Math.min(Math.max((value() / max) * 100, 0), 100)) | |
| 79 | const percentage = derived(() => Math.min(Math.max((value() / maxValue()) * 100, 0), 100)) | |
| 57 | 80 | const percentLabel = derived(() => `${Math.round(percentage())}%`) |
| 58 | const linearWidth = derived(() => indeterminate ? '30%' : `${percentage()}%`) | |
| 59 | const dashOffset = derived(() => indeterminate ? 0 : circumference - (percentage() / 100) * circumference) | |
| 81 | const linearWidth = derived(() => isIndeterminate() ? '30%' : `${percentage()}%`) | |
| 82 | const dashOffset = derived(() => isIndeterminate() ? 0 : circumference - (percentage() / 100) * circumference) | |
| 83 | // The spin is part of what `indeterminate` MEANS, so it follows the signal | |
| 84 | // rather than the server's snapshot of it. | |
| 85 | const circularBarClass = derived(() => `${circularBarColor} transition-all duration-300 ease-in-out ${isIndeterminate() ? 'animate-spin' : ''}`.trim()) | |
| 60 | 86 | |
| 61 | 87 | defineExpose({ value, setValue: (v) => value.set(v) }) |
| 62 | 88 | </script> |
| @@ -74,6 +100,7 @@ defineExpose({ value, setValue: (v) => value.set(v) }) | ||
| 74 | 100 | :aria-valuenow="value()" |
| 75 | 101 | aria-valuemin="0" |
| 76 | 102 | aria-valuemax="function max(array) { if (array.length === 0) throw Error("Cannot compute max of an empty array"); return Math.max(...array); }" |
| 103 | :aria-valuemax="maxValue()" | |
| 77 | 104 | > |
| 78 | 105 | <div |
| 79 | 106 | class="" |
| @@ -93,6 +120,7 @@ defineExpose({ value, setValue: (v) => value.set(v) }) | ||
| 93 | 120 | :aria-valuenow="value()" |
| 94 | 121 | aria-valuemin="0" |
| 95 | 122 | aria-valuemax="function max(array) { if (array.length === 0) throw Error("Cannot compute max of an empty array"); return Math.max(...array); }" |
| 123 | :aria-valuemax="maxValue()" | |
| 96 | 124 | > |
| 97 | 125 | <circle |
| 98 | 126 | class="" |
| @@ -104,6 +132,7 @@ defineExpose({ value, setValue: (v) => value.set(v) }) | ||
| 104 | 132 | /> |
| 105 | 133 | <circle |
| 106 | 134 | class=" transition-all duration-300 ease-in-out " |
| 135 | :class="circularBarClass()" | |
| 107 | 136 | cx="NaN" |
| 108 | 137 | cy="NaN" |
| 109 | 138 | r="" |
| @@ -150,6 +179,7 @@ bg-purple-600 | ||
| 150 | 179 | bg-success-solid |
| 151 | 180 | bg-surface-sunken |
| 152 | 181 | bg-yellow-600 |
| 182 | circularBarClass() | |
| 153 | 183 | circularBarColor |
| 154 | 184 | circularSizeClass |
| 155 | 185 | circularTrackColor |
| @@ -182,6 +212,7 @@ justify-center | ||
| 182 | 212 | labelClasses |
| 183 | 213 | linearBarClasses |
| 184 | 214 | linearTrackClasses |
| 215 | max | |
| 185 | 216 | mb-2 |
| 186 | 217 | overflow-hidden |
| 187 | 218 | progressbar |
| @@ -31,13 +31,16 @@ export const arrowClass = `absolute w-0 h-0 border-4 border-transparent ${arrowC | ||
| 31 | 31 | </script> |
| 32 | 32 | <script client> |
| 33 | 33 | const delay = function delay(ms) { if (ms < 0 || !Number.isInteger(ms)) throw Error("delay() requires a non-negative integer"); return new Promise((resolve) => setTimeout(resolve, ms)); } |
| 34 | const isDisabled = | |
| 34 | // A live prop: a tooltip is commonly suppressed by state the page only learns | |
| 35 | // about later — while a field is being edited, or once a hint has been read — | |
| 36 | // and frozen it could only ever reflect the state the page was built in. | |
| 37 | const isDisabled = useReactiveProp('disabled', ) | |
| 35 | 38 | // Reactive prop binding — parent can drive `show` via signals. See #1704. |
| 36 | 39 | const isVisible = useReactiveProp('show', ) |
| 37 | 40 | let timeoutId = null |
| 38 | 41 | |
| 39 | 42 | function showTooltip() { |
| 40 | if (isDisabled) return | |
| 43 | if (isDisabled()) return | |
| 41 | 44 | if (delay > 0) { |
| 42 | 45 | timeoutId = setTimeout(() => isVisible.set(true), delay) |
| 43 | 46 | } |