boban
/
CluPilotCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Say that a click landed, and make moving one actually move it
tests / pest (push) Failing after 7m52s Details
tests / assets (push) Successful in 20s Details
tests / release (push) Has been skipped Details 

"Man klickt und sieht nicht was passiert." Every action in this console is
a round trip, and a round trip with no sign of itself reads as a click that
missed — so the answer is in three places, from the general to the
specific.

A thin bar at the top of the window for any Livewire request, in app.js.
Delayed by 140 ms, because a request that answers faster than that is
perceived as immediate and a bar flashing on every keystroke of a .live
field is worse than none. It creeps to 70 % and only completes on the
answer: a bar that reaches the end while the answer is still on its way is
a lie about the thing it exists to report. Released on failure as well —
the request somebody must not be left waiting on is precisely the one that
went wrong — and on wire:navigate, which is not a request hook at all and
would otherwise leave the bar behind on a page that is gone.

Per button: a pressed state on the way down, disabled while its own request
is out, scoped with wire:target to that button's own call so a row does not
grey itself out because a neighbour is moving.

The edit button carries its own spinner, because opening the modal is a
request made by the MODAL component — wire:loading on this page never sees
it. It clears when the package announces the modal is up, and on a six
second timer as well, so a request that never answers cannot leave a row
spinning for ever.

And the reordering was not only slow to look at, it was wrong. It added or
subtracted a fixed amount: with the seeded values ten apart that usually
landed right, and sometimes landed on a value another template already had
— or on the floor along with three others. Equal sort values fall back to
the name, so a click could reorder something else, or nothing, which is
indistinguishable from a click that never registered. It swaps with the
neighbour now: exactly one place, or nothing at the ends, both of which are
what an operator expects to see. Where two shared a value the swap leaves
them distinct, or the next click would do nothing again.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feature/betriebsmodus v1.3.40
nexxo 2026-07-29 23:06:30 +02:00
parent 173f05c1bd
commit 57c6912987
5 changed files with 261 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
VERSION
View File

@ -1 +1 @@
1.3.39
1.3.40

53
app/Livewire/Admin/MailTemplates.php
View File

@ -78,16 +78,61 @@ class MailTemplates extends Component
}
}
/** Move one up or down the list an operator picks from. */
public function move(string $uuid, int $by): void
/**
* Swap one with its neighbour.
*
* It used to add or subtract a fixed amount, which is not the same thing:
* with the seeded values ten apart, ±15 usually landed correctly — and
* sometimes landed on a value another template already had, or on the 0
* floor along with three others. Equal `sort` values fall back to the name,
* so a click could reorder something else, or visibly nothing, which is
* indistinguishable from a click that never registered.
*
* A swap always moves exactly one place, or does nothing at the ends, and
* both of those are what the operator expects to see.
*/
public function move(string $uuid, int $direction): void
{
$this->authorize('customers.manage');
$template = MailTemplate::query()->where('uuid', $uuid)->first();
if ($template !== null) {
$template->update(['sort' => max(0, $template->sort + $by)]);
if ($template === null) {
return;
}
// The one immediately above or below in the order the page shows —
// read through the same ordering, so the swap agrees with the list.
$neighbour = MailTemplate::query()
->when($direction < 0, fn ($q) => $q
->where(fn ($w) => $w->where('sort', '<', $template->sort)
->orWhere(fn ($t) => $t->where('sort', $template->sort)->where('name', '<', $template->name)))
->orderByDesc('sort')->orderByDesc('name'))
->when($direction > 0, fn ($q) => $q
->where(fn ($w) => $w->where('sort', '>', $template->sort)
->orWhere(fn ($t) => $t->where('sort', $template->sort)->where('name', '>', $template->name)))
->orderBy('sort')->orderBy('name'))
->first();
if ($neighbour === null) {
return; // already at the end; nothing to swap with
}
// Distinct values, even where the two were equal before: leaving them
// equal would put the order back in the hands of the name.
$mine = $template->sort;
$theirs = $neighbour->sort;
if ($mine === $theirs) {
// Moving UP means taking the lower value, so the one the mover
// receives is below the neighbour's — not above it. The other way
// round swaps them into the order they were already in, which looks
// exactly like the bug this method was rewritten to fix.
$theirs = $mine + ($direction < 0 ? -1 : 1);
}
$template->update(['sort' => $theirs]);
$neighbour->update(['sort' => $mine]);
}
public function render()

95
resources/js/app.js
View File

@ -131,8 +131,96 @@ function recoverFrom419() {
window.location.reload();
}
// A thin bar at the top of the window while a Livewire request is in flight.
//
// Reported as "man klickt und sieht nicht was passiert": every action in the
// console is a round trip, and a round trip with no sign of itself reads as a
// click that missed. wire:loading fixes that per element, one element at a time,
// and only where somebody remembered to add it — so this is the floor underneath
// all of them.
//
// Deliberately delayed: a request that answers in 80 ms does not need a bar, and
// flashing one for every keystroke on a .live field is worse than nothing.
// SHOW_AFTER_MS is the threshold below which a person perceives the response as
// immediate anyway.
const BUSY_SHOW_AFTER_MS = 140;
const BUSY_FADE_MS = 220;
function requestIndicator() {
let bar = null;
let showTimer = null;
let inFlight = 0;
const element = () => {
if (bar !== null) return bar;
bar = document.createElement('div');
// Inline styles, not a class: this has to work on the login page, on the
// 503 page, and anywhere else the app's stylesheet may not have loaded
// the utilities it would otherwise need.
bar.style.cssText = [
'position:fixed', 'top:0', 'left:0', 'height:2px', 'width:0%',
'background:#c2560a', 'z-index:2147483647', 'pointer-events:none',
'transition:width .25s ease-out,opacity .22s linear', 'opacity:0',
].join(';');
bar.setAttribute('aria-hidden', 'true');
document.body.appendChild(bar);
return bar;
};
const show = () => {
const el = element();
el.style.opacity = '1';
// Not to 100: the bar must never claim to be finished while the answer
// is still on its way. It creeps and then completes.
el.style.width = '70%';
};
const hide = () => {
if (bar === null) return;
bar.style.width = '100%';
bar.style.opacity = '0';
window.setTimeout(() => {
if (inFlight === 0 && bar !== null) bar.style.width = '0%';
}, BUSY_FADE_MS);
};
return {
start() {
inFlight++;
if (showTimer !== null) return;
showTimer = window.setTimeout(() => {
showTimer = null;
if (inFlight > 0) show();
}, BUSY_SHOW_AFTER_MS);
},
// Called for success AND failure: a request that fails is exactly the
// one somebody must not be left waiting on.
stop() {
inFlight = Math.max(0, inFlight - 1);
if (inFlight > 0) return;
if (showTimer !== null) {
window.clearTimeout(showTimer);
showTimer = null;
}
hide();
},
};
}
const busy = requestIndicator();
document.addEventListener('livewire:init', () => {
window.Livewire.hook('request', ({ fail }) => {
window.Livewire.hook('request', ({ fail, respond }) => {
busy.start();
// respond fires for every answer, whatever its status; fail() below
// handles the ones worth acting on. Both paths must release the bar,
// and respond runs first — so stopping here covers the failures too.
respond(() => busy.stop());
fail(({ status, preventDefault }) => {
// 503: the application is in maintenance mode, which during a
// deployment is not a fault — it is the event being watched.
@ -159,6 +247,11 @@ document.addEventListener('livewire:init', () => {
});
});
// A navigation (wire:navigate) is not a request hook, and the bar would
// otherwise sit there after the page it belonged to is gone.
document.addEventListener('livewire:navigating', () => busy.start());
document.addEventListener('livewire:navigated', () => busy.stop());
// Connection banner ("Keine Verbindung" / "Verbindung wiederhergestellt" —
// see connectionBanner() below): navigator.onLine and the browser's
// online/offline events are both used, but neither is trusted alone —

46
resources/views/livewire/admin/mail-templates.blade.php
View File

@ -84,7 +84,17 @@
@else
<div class="grid gap-3 lg:grid-cols-2 animate-rise [animation-delay:120ms]">
@foreach ($templates as $template)
{{-- `opening` is local to the row: fetching the modal is a round
trip, and a click with nothing to show for it reads as a
click that missed. It clears when the modal is up (the
package announces that) and on a timer as well, so a request
that never answers cannot leave the row spinning for ever. --}}
<div wire:key="tpl-{{ $template->uuid }}"
x-data="{ opening: false }"
x-on:modal-opened.window="opening = false"
x-on:modalclosed.window="opening = false"
wire:loading.class="opacity-60"
wire:target="move('{{ $template->uuid }}', -1), move('{{ $template->uuid }}', 1), toggleActive('{{ $template->uuid }}')"
@class([
'group flex items-center gap-3 rounded-lg border bg-surface px-4 py-3 shadow-xs transition-colors',
'border-line hover:border-accent-border' => $template->active,
@ -105,29 +115,43 @@
<p class="truncate text-xs text-muted">{{ $template->subject }}</p>
</div>
{{-- Icons, and one word for the state change. Two full-width
buttons per entry was most of the height. --}}
{{-- Every button reports its own click: pressed state on
the way down, disabled while the request is out. The
global bar at the top of the window says that SOMETHING
is happening; these say which thing. --}}
<div class="flex shrink-0 items-center gap-0.5">
<button type="button" wire:click="move('{{ $template->uuid }}', -15)"
class="rounded p-1.5 text-faint hover:text-ink" aria-label="{{ __('templates.up') }}" title="{{ __('templates.up') }}">
<button type="button" wire:click="move('{{ $template->uuid }}', -1)"
wire:loading.attr="disabled" wire:target="move('{{ $template->uuid }}', -1)"
class="rounded p-1.5 text-faint transition-colors hover:bg-surface-hover hover:text-ink active:bg-accent-subtle active:text-accent-text disabled:opacity-40"
aria-label="{{ __('templates.up') }}" title="{{ __('templates.up') }}">
<x-ui.icon name="chevron-down" class="size-4 rotate-180" />
</button>
<button type="button" wire:click="move('{{ $template->uuid }}', 15)"
class="rounded p-1.5 text-faint hover:text-ink" aria-label="{{ __('templates.down') }}" title="{{ __('templates.down') }}">
<button type="button" wire:click="move('{{ $template->uuid }}', 1)"
wire:loading.attr="disabled" wire:target="move('{{ $template->uuid }}', 1)"
class="rounded p-1.5 text-faint transition-colors hover:bg-surface-hover hover:text-ink active:bg-accent-subtle active:text-accent-text disabled:opacity-40"
aria-label="{{ __('templates.down') }}" title="{{ __('templates.down') }}">
<x-ui.icon name="chevron-down" class="size-4" />
</button>
<button type="button" wire:click="toggleActive('{{ $template->uuid }}')"
class="rounded p-1.5 text-faint hover:text-warning"
wire:loading.attr="disabled" wire:target="toggleActive('{{ $template->uuid }}')"
class="rounded p-1.5 text-faint transition-colors hover:bg-surface-hover hover:text-warning active:bg-warning-bg disabled:opacity-40"
aria-label="{{ $template->active ? __('templates.retire') : __('templates.restore') }}"
title="{{ $template->active ? __('templates.retire') : __('templates.restore') }}">
<x-ui.icon :name="$template->active ? 'x' : 'refresh'" class="size-4" />
</button>
{{-- R20: editing opens a modal. A five-line textarea in a
list row is the height jump the rule exists to stop. --}}
list row is the height jump the rule exists to stop.
The spinner is this button's own: opening the modal
is a Livewire request made by the MODAL component,
so wire:loading on this page never sees it. --}}
<button type="button"
x-on:click="$dispatch('openModal', { component: 'admin.edit-mail-template', arguments: { uuid: '{{ $template->uuid }}' } })"
class="rounded p-1.5 text-muted hover:text-ink" aria-label="{{ __('templates.edit') }}" title="{{ __('templates.edit') }}">
<x-ui.icon name="pencil" class="size-4" />
x-bind:disabled="opening"
x-on:click="opening = true; setTimeout(() => opening = false, 6000); $dispatch('openModal', { component: 'admin.edit-mail-template', arguments: { uuid: '{{ $template->uuid }}' } })"
class="rounded p-1.5 text-muted transition-colors hover:bg-surface-hover hover:text-ink active:bg-accent-subtle active:text-accent-text disabled:opacity-60"
aria-label="{{ __('templates.edit') }}" title="{{ __('templates.edit') }}">
<x-ui.icon name="pencil" class="size-4" x-show="!opening" />
<x-ui.icon name="refresh" class="size-4 animate-spin text-accent-text" x-show="opening" x-cloak />
</button>
</div>
</div>

82
tests/Feature/Admin/MailTemplateTest.php
View File

@ -10,6 +10,7 @@ use App\Models\SentMail;
use App\Models\Subscription;
use App\Services\Mail\MailTemplateRenderer;
use App\Support\CompanyProfile;
use Illuminate\Support\Facades\File;
use Livewire\Livewire;
/**
@ -302,3 +303,84 @@ it('leaves an edited template alone when the seed runs again', function () {
expect($template->fresh()->body)->toBe('Meine eigene Fassung.')
->and(MailTemplate::query()->where('name', 'Anfrage bestätigen')->count())->toBe(1);
});
// ---- Moving one, and seeing that it moved ----
it('swaps a template with its neighbour rather than nudging a number', function () {
// It used to add or subtract a fixed amount, which is not the same thing:
// with values ten apart that usually landed right, and sometimes landed on a
// value another template already had — equal sort falls back to the name, so
// a click could reorder something else, or visibly nothing at all, which is
// indistinguishable from a click that never registered.
MailTemplate::query()->delete();
$first = MailTemplate::create(['name' => 'A', 'subject' => 'S', 'body' => 'B', 'sort' => 10]);
$second = MailTemplate::create(['name' => 'B', 'subject' => 'S', 'body' => 'B', 'sort' => 20]);
$third = MailTemplate::create(['name' => 'C', 'subject' => 'S', 'body' => 'B', 'sort' => 30]);
$order = fn () => MailTemplate::query()->orderBy('sort')->orderBy('name')->pluck('name')->all();
$page = Livewire::actingAs(operator('Owner'), 'operator')->test(MailTemplates::class);
$page->call('move', $third->uuid, -1);
expect($order())->toBe(['A', 'C', 'B']);
$page->call('move', $third->uuid, -1);
expect($order())->toBe(['C', 'A', 'B']);
$page->call('move', $third->uuid, 1);
expect($order())->toBe(['A', 'C', 'B']);
});
it('does nothing at the ends instead of pretending', function () {
// Both are what an operator expects to see: the top one cannot go up. A
// number that silently changed without the order changing was the bug.
MailTemplate::query()->delete();
$top = MailTemplate::create(['name' => 'A', 'subject' => 'S', 'body' => 'B', 'sort' => 10]);
$bottom = MailTemplate::create(['name' => 'B', 'subject' => 'S', 'body' => 'B', 'sort' => 20]);
$page = Livewire::actingAs(operator('Owner'), 'operator')->test(MailTemplates::class);
$page->call('move', $top->uuid, -1);
$page->call('move', $bottom->uuid, 1);
expect(MailTemplate::query()->orderBy('sort')->pluck('name')->all())->toBe(['A', 'B'])
->and($top->fresh()->sort)->toBe(10)
->and($bottom->fresh()->sort)->toBe(20);
});
it('separates two templates that shared a sort value', function () {
// Equal values put the order in the hands of the name, so a swap between
// them has to leave them distinct or the next click does nothing.
MailTemplate::query()->delete();
$a = MailTemplate::create(['name' => 'A', 'subject' => 'S', 'body' => 'B', 'sort' => 5]);
$b = MailTemplate::create(['name' => 'B', 'subject' => 'S', 'body' => 'B', 'sort' => 5]);
Livewire::actingAs(operator('Owner'), 'operator')
->test(MailTemplates::class)
->call('move', $b->uuid, -1);
expect($a->fresh()->sort)->not->toBe($b->fresh()->sort)
->and(MailTemplate::query()->orderBy('sort')->orderBy('name')->pluck('name')->all())->toBe(['B', 'A']);
});
it('says a click landed, on the button and at the top of the window', function () {
// Reported as "man klickt und sieht nicht was passiert". Every action here is
// a round trip; a round trip with no sign of itself reads as a click that
// missed. The bar is the floor under every page, the per-button state says
// WHICH thing is happening.
$page = File::get(resource_path('views/livewire/admin/mail-templates.blade.php'));
expect($page)->toContain('wire:loading.attr="disabled"')
->toContain('animate-spin')
// Scoped to the one button, not to the whole page: every row disabling
// itself because a neighbour is moving is its own kind of confusing.
->toContain('wire:target="move(');
expect(File::get(resource_path('js/app.js')))
->toContain('BUSY_SHOW_AFTER_MS')
// Released on failure too — the request somebody must not be left
// waiting on is exactly the one that went wrong.
->toContain('respond(() => busy.stop())');
});