also looking at this
fix(components): Button reads disabled and loading as live props
#2007
Merged
glennmichael123
wants to merge
fix/button-reactive-disabled-loading
into
main
3 files
+255
-25
| @@ -37,10 +37,39 @@ export const spinnerClasses = `animate-spin ${iconClasses}`.trim() | ||
| 37 | 37 | |
| 38 | 38 | <script client> |
| 39 | 39 | const emit = defineEmits() |
| 40 | -const disabledNow = {{ isDisabled }} | |
| 40 | + | |
| 41 | +/* | |
| 42 | + * `disabled` and `loading` are read as live props, not as a server snapshot. | |
| 43 | + * | |
| 44 | + * This used to be `const disabledNow = ` — a value computed | |
| 45 | + * before the page was sent and never looked at again. So the most common piece | |
| 46 | + * of button state in an application could not be expressed: | |
| 47 | + * | |
| 48 | + * <Button :disabled="saving()" @click="save">Save</Button> | |
| 49 | + * | |
| 50 | + * `saving()` is a signal. On the server it is not yet meaningful, so the button | |
| 51 | + * shipped enabled and STAYED enabled for the life of the page — through the | |
| 52 | + * exact async action it was there to guard. Double submits, destructive | |
| 53 | + * deletes and rotate-key confirmations all land on this. | |
| 54 | + * | |
| 55 | + * useReactiveProp wires the parent's binding into a local signal: the parent's | |
| 56 | + * bindAttr writes the attribute on our root, a MutationObserver picks it up, | |
| 57 | + * and the signal flips. Dialog already does this for `open` (#1704) and Switch | |
| 58 | + * for `checked`, so this is the package's existing answer rather than a new one. | |
| 59 | + * | |
| 60 | + * See stacksjs/stx#1997. | |
| 61 | + */ | |
| 62 | +const isLoading = useReactiveProp('loading', {{ loading }}) | |
| 63 | +const disabledProp = useReactiveProp('disabled', {{ disabled }}) | |
| 64 | + | |
| 65 | +// Loading implies disabled, the same combination the server computes for the | |
| 66 | +// initial render, so the two agree before and after hydration. | |
| 67 | +const isDisabled = derived(() => disabledProp() || isLoading()) | |
| 41 | 68 | |
| 42 | 69 | function onClick(event) { |
| 43 | - if (disabledNow) { | |
| 70 | + // Read through the signal: a button disabled after render must stop guarding | |
| 71 | + // on the live value, not on what was true when the page was built. | |
| 72 | + if (isDisabled()) { | |
| 44 | 73 | event.preventDefault() |
| 45 | 74 | return |
| 46 | 75 | } |
| @@ -52,23 +81,35 @@ function onClick(event) { | ||
| 52 | 81 | type="" |
| 53 | 82 | class="" |
| 54 | 83 | @if(isDisabled)disabled@endif |
| 55 | - @click="onClick($event)" | |
| 84 | + :disabled="isDisabled()" | |
| 56 | 85 | aria-busy="false" |
| 86 | + :aria-busy="isLoading() ? 'true' : 'false'" | |
| 87 | + @click="onClick($event)" | |
| 57 | 88 | > |
| 58 | - @if(loading) | |
| 59 | - <svg class="" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24"> | |
| 60 | - <circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4"></circle> | |
| 61 | - <path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z"></path> | |
| 62 | - </svg> | |
| 63 | - @else | |
| 64 | - @if(leftIcon) | |
| 65 | - <span class="">{!! leftIcon !!}</span> | |
| 66 | - @endif | |
| 89 | + {{-- The spinner is always in the DOM so `loading` can turn it on after render; | |
| 90 | + it was inside an @if, which meant the only way to see it was to have been | |
| 91 | + loading when the page was built. x-cloak only when the server says we are | |
| 92 | + NOT loading, so a button rendered loading shows its spinner on first | |
| 93 | + paint rather than waiting for hydration. --}} | |
| 94 | + <svg | |
| 95 | + :show="isLoading()" | |
| 96 | + @if(!loading)x-cloak@endif | |
| 97 | + class="" | |
| 98 | + xmlns="http://www.w3.org/2000/svg" | |
| 99 | + fill="none" | |
| 100 | + viewBox="0 0 24 24" | |
| 101 | + > | |
| 102 | + <circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4"></circle> | |
| 103 | + <path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z"></path> | |
| 104 | + </svg> | |
| 105 | + | |
| 106 | + @if(leftIcon) | |
| 107 | + <span :show="!isLoading()" @if(loading)x-cloak@endif class="">{!! leftIcon !!}</span> | |
| 67 | 108 | @endif |
| 68 | 109 | |
| 69 | 110 | <slot /> |
| 70 | 111 | |
| 71 | - @if(!loading && rightIcon) | |
| 72 | - <span class="">{!! rightIcon !!}</span> | |
| 112 | + @if(rightIcon) | |
| 113 | + <span :show="!isLoading()" @if(loading)x-cloak@endif class="">{!! rightIcon !!}</span> | |
| 73 | 114 | @endif |
| 74 | 115 | </button> |