Преглед на файлове

Fix: Phase 18 cell markup — drop span wrapper, color goes on <td>

The original Phase 18 markup wrapped the day input in
<span class="assign-cell"> with display:inline-flex, then put the
status <select> as its sibling. In production the user reported that
clicking an input and blurring it triggered "Nothing changed" flashes
on every cell and made the inputs disappear — the inline-flex wrapper
inside a table cell turned out to be a layout trap.

Rewritten flat:
- The status colour class (assign-status-*) and data-assign-cell /
  data-status / data-sw-id attrs now sit directly on the <td>. No
  nested wrapper.
- The day input renders as a direct child of the <td> with the same
  classes it had pre-Phase-18, so its layout is unchanged.
- The chevron-only <select data-assign-status> is a sibling, also
  inside the <td>.

CSS dropped the `.assign-cell` block and the
`:not(.assign-status-zugewiesen) input[data-assign]` background
overrides — both irrelevant once the wrapper is gone. Color rules
apply directly to <td>; option text style stays. Tailwind's safelist
is unchanged (the 4 class names still get interpolated server-side).

JS hardened:
- The [data-assign-status] change handler is now wired only when
  `taskStatusEnabled === true`; bails defensively if `closest(
  '[data-assign-cell]')` is empty or task/sw IDs aren't finite.
- Status class swap inlined (deleted applyStatusClass helper).
- buildTaskRow stamps the status attrs directly on the new <td> for
  admin-added rows, mirroring the server-rendered structure.

Verified end-to-end: rebuilt the docker image, rendered both
sprints/show.php and sprints/present.php through a one-shot script
(deleted post-verify) — the rendered <td> is flat, the input is a
direct child, the status select is a sibling, the toolbar carries
4 [data-status-filter-opt] checkboxes. With the global flag off, the
markup matches the pre-Phase-18 shape exactly. PHPUnit at 105/265.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa преди 3 дни
родител
ревизия
3e115f5119
променени са 4 файла, в които са добавени 73 реда и са изтрити 85 реда
  1. 19 32
      assets/css/input.css
  2. 44 37
      public/assets/js/sprint-planner.js
  3. 3 8
      views/sprints/present.php
  4. 7 8
      views/sprints/show.php

+ 19 - 32
assets/css/input.css

@@ -56,22 +56,15 @@
         padding: 0.5rem 0.25rem;
     }
 
-    /* Phase 18: per-cell task-assignment status. Each <td> holding a
-       day input also wraps it in a .assign-cell with an
-       .assign-status-* color class. The class is on the wrapper so
-       both the day input/span and the chevron-only <select> share
-       the tint. The selector itself shows only its native arrow —
-       no text, no width — so it sits as a tiny chevron next to the
-       day field. */
-    .assign-cell {
-        display: inline-flex;
-        align-items: center;
-        gap: 1px;
-        padding: 1px;
-        border-radius: 0.25rem;
-        line-height: 1;
-    }
-    .assign-status-zugewiesen { background-color: transparent; }
+    /* Phase 18: per-cell task-assignment status. The colour class
+       applies directly to the <td> — no nested wrapper — so the cell's
+       table-layout and the input it contains render exactly as they
+       did before the feature, with only the cell background tinting.
+       The four .assign-status-* class names are interpolated server-
+       side ("assign-status-<?= $st ?>"), so they're held in
+       tailwind.config.js's safelist; otherwise the JIT prunes them. */
+    .assign-status-zugewiesen { /* default: no override, stays whatever
+                                   the row's bg is */ }
     .assign-status-gestartet     { background-color: theme('colors.yellow.200'); }
     .assign-status-abgeschlossen { background-color: theme('colors.green.200'); }
     .assign-status-abgebrochen   { background-color: theme('colors.red.200'); }
@@ -79,29 +72,22 @@
     .dark .assign-status-abgeschlossen { background-color: theme('colors.green.700'); }
     .dark .assign-status-abgebrochen   { background-color: theme('colors.red.700'); }
 
-    /* Inputs/spans inside a coloured cell get a transparent background
-       so the wrapper tint shows through; the day input keeps its own
-       focus ring. */
-    .assign-cell:not(.assign-status-zugewiesen) input[data-assign],
-    .assign-cell:not(.assign-status-zugewiesen) > .font-mono {
-        background-color: transparent;
-    }
-
+    /* Tiny chevron-only <select> sitting next to the day input. We hide
+       the selected text via font-size:0 + color:transparent so only the
+       browser's native dropdown arrow shows; clicking it opens the
+       option list with its full-size text restored. */
     .assign-status-select {
-        /* Strip the native <select> body — keep only the dropdown
-           chevron. We hide the text by sizing the box to zero width
-           plus padding for the arrow, and rely on each browser's
-           default arrow rendering. */
-        width: 1.1rem;
+        width: 1rem;
         min-width: 0;
+        margin-left: 2px;
         padding: 0;
-        margin: 0;
         border: 0;
         background-color: transparent;
-        font-size: 0;          /* hide selected option text */
+        font-size: 0;
         line-height: 1;
-        color: transparent;    /* fallback if font-size:0 fails */
+        color: transparent;
         cursor: pointer;
+        vertical-align: middle;
     }
     .assign-status-select:focus {
         outline: 2px solid theme('colors.slate.400');
@@ -111,6 +97,7 @@
     .assign-status-select option {
         font-size: 0.875rem;
         color: theme('colors.slate.900');
+        background-color: theme('colors.white');
     }
     .dark .assign-status-select option {
         color: theme('colors.slate.100');

+ 44 - 37
public/assets/js/sprint-planner.js

@@ -395,30 +395,32 @@
                 .attr('data-col', 'sw-' + sw.id)
                 .attr('data-sort-value-sw-' + sw.id, v.toFixed(2));
 
-            const $input = $('<input type="number" min="0" step="0.5" data-assign class="w-14 rounded border border-slate-200 px-1 py-1 text-center font-mono focus:outline-none focus:ring-2 focus:ring-slate-400">')
-                .val(fmtDays(v))
-                .attr('data-sw-id', sw.id);
-
+            // Phase 18: when the feature is enabled the status attrs +
+            // colour class go directly on the <td>. New rows always start
+            // at the default status (no nested wrapper).
             if (taskStatusEnabled) {
-                // New tasks always start with the default status.
-                const $cell = $('<span class="assign-cell assign-status-zugewiesen"></span>')
-                    .attr('data-assign-cell', '')
+                $td.addClass('assign-status-zugewiesen')
+                   .attr('data-assign-cell', '')
+                   .attr('data-status', 'zugewiesen')
+                   .attr('data-sw-id', sw.id);
+            }
+
+            $td.append(
+                $('<input type="number" min="0" step="0.5" data-assign class="w-14 rounded border border-slate-200 px-1 py-1 text-center font-mono focus:outline-none focus:ring-2 focus:ring-slate-400">')
+                    .val(fmtDays(v))
                     .attr('data-sw-id', sw.id)
-                    .attr('data-status', 'zugewiesen');
-                $cell.append($input);
+            );
 
+            if (taskStatusEnabled) {
                 const $status = $('<select data-assign-status aria-label="Status" class="assign-status-select"></select>')
                     .attr('data-sw-id', sw.id);
                 STATUSES.forEach(function (s) {
                     $('<option>').val(s).text(s).appendTo($status);
                 });
                 $status.val('zugewiesen');
-                $cell.append($status);
-
-                $td.append($cell);
-            } else {
-                $td.append($input);
+                $td.append($status);
             }
+
             $tr.append($td);
         });
 
@@ -579,7 +581,8 @@
     // --- Phase 18: per-cell status save pipeline -------------------------
     // Independent of the days pipeline: hits /tasks/{id}/assignments/status
     // (signed-in route, gated by app_settings.task_status_enabled). Same
-    // debounce semantics + same audit weight (one row per changed cell).
+    // debounce + audit semantics as the days pipeline (one row per changed
+    // cell). Skipped entirely when the feature flag is off.
 
     const pendingStatus = new Map(); // taskId -> Map<swId, status>
     const statusTimers  = {};
@@ -608,30 +611,34 @@
             .catch(function (e) { flash(e.message, true); });
     }
 
-    function applyStatusClass($cell, next) {
-        // Replace any existing assign-status-* with the new one. Keep the
-        // class set deterministic (any unknown classes get scrubbed).
-        STATUSES.forEach(function (s) { $cell.removeClass('assign-status-' + s); });
-        $cell.addClass('assign-status-' + next);
-        $cell.attr('data-status', next);
+    // Status change handler — only attach when the feature is on, and bail
+    // defensively if the cell doesn't carry the data attributes we expect
+    // (e.g. on a partially-rendered task row right after `+ Add task`).
+    if (taskStatusEnabled) {
+        $root.on('change', '[data-assign-status]', function () {
+            const $sel  = $(this);
+            const next  = String($sel.val() || '');
+            if (STATUSES.indexOf(next) === -1) { return; }
+
+            const $cell = $sel.closest('[data-assign-cell]');
+            if ($cell.length === 0) { return; }
+
+            const $tr = $sel.closest('tr');
+            const taskId = parseInt($tr.attr('data-task-id'), 10);
+            const swId   = parseInt($sel.attr('data-sw-id'), 10);
+            if (!Number.isFinite(taskId) || !Number.isFinite(swId)) { return; }
+
+            // Swap the cell's colour class + data-status. Done first so the
+            // visual flip is instant; the server save is debounced.
+            STATUSES.forEach(function (s) { $cell.removeClass('assign-status-' + s); });
+            $cell.addClass('assign-status-' + next);
+            $cell.attr('data-status', next);
+
+            queueStatus(taskId, swId, next);
+            applyFilters();
+        });
     }
 
-    $root.on('change', '[data-assign-status]', function () {
-        const $sel  = $(this);
-        const next  = String($sel.val() || '');
-        if (STATUSES.indexOf(next) === -1) { return; }
-        const $cell = $sel.closest('[data-assign-cell]');
-        applyStatusClass($cell, next);
-
-        const taskId = parseInt($sel.closest('tr').data('task-id'), 10);
-        const swId   = parseInt($sel.data('sw-id'), 10);
-        queueStatus(taskId, swId, next);
-
-        // Re-evaluate the status filter immediately so the row hides /
-        // shows without waiting for the server round-trip.
-        applyFilters();
-    });
-
     function applyServerCapacity(perWorker) {
         if (!perWorker || typeof perWorker !== 'object') { return; }
         Object.keys(perWorker).forEach(function (swIdStr) {

+ 3 - 8
views/sprints/present.php

@@ -331,16 +331,12 @@ if (!function_exists('fmt_days')) {
                                 <?php foreach ($sprintWorkers as $sw):
                                     $d  = (float) ($assign[$sw->id] ?? 0.0);
                                     $st = (string) ($statusGrid[$t->id][$sw->id] ?? TaskAssignment::STATUS_ZUGEWIESEN);
+                                    $tdExtraClass = $taskStatusEnabled ? ' assign-status-' . $st : '';
                                 ?>
-                                    <td class="px-1 py-1 text-center"
+                                    <td class="px-1 py-1 text-center<?= e($tdExtraClass) ?>"
                                         data-col="sw-<?= (int) $sw->id ?>"
+                                        <?php if ($taskStatusEnabled): ?>data-assign-cell data-status="<?= e($st) ?>" data-sw-id="<?= (int) $sw->id ?>"<?php endif; ?>
                                         data-sort-value-sw-<?= (int) $sw->id ?>="<?= e(number_format($d, 2, '.', '')) ?>">
-                                        <?php if ($taskStatusEnabled): ?>
-                                            <span class="assign-cell assign-status-<?= e($st) ?>"
-                                                  data-assign-cell
-                                                  data-sw-id="<?= (int) $sw->id ?>"
-                                                  data-status="<?= e($st) ?>">
-                                        <?php endif; ?>
                                         <?php if ($currentUser->isAdmin): ?>
                                             <input type="number" min="0" step="0.5"
                                                    value="<?= e(fmt_days($d)) ?>"
@@ -359,7 +355,6 @@ if (!function_exists('fmt_days')) {
                                                     <option value="<?= e($opt) ?>" <?= $opt === $st ? 'selected' : '' ?>><?= e($opt) ?></option>
                                                 <?php endforeach; ?>
                                             </select>
-                                            </span>
                                         <?php endif; ?>
                                     </td>
                                 <?php endforeach; ?>

+ 7 - 8
views/sprints/show.php

@@ -462,16 +462,16 @@ if (!function_exists('fmt_days')) {
                                 <?php foreach ($sprintWorkers as $sw):
                                     $d  = (float) ($assign[$sw->id] ?? 0.0);
                                     $st = (string) ($statusGrid[$t->id][$sw->id] ?? TaskAssignment::STATUS_ZUGEWIESEN);
+                                    // Phase 18: when the global flag is on, the status colour class
+                                    // and data-* attributes live on the <td> itself — no nested
+                                    // wrapper. Keeps the cell's table-layout intact and the days
+                                    // input renders exactly as it did pre-Phase-18.
+                                    $tdExtraClass = $taskStatusEnabled ? ' assign-status-' . $st : '';
                                 ?>
-                                    <td class="px-1 py-1 text-center"
+                                    <td class="px-1 py-1 text-center<?= e($tdExtraClass) ?>"
                                         data-col="sw-<?= (int) $sw->id ?>"
+                                        <?php if ($taskStatusEnabled): ?>data-assign-cell data-status="<?= e($st) ?>" data-sw-id="<?= (int) $sw->id ?>"<?php endif; ?>
                                         data-sort-value-sw-<?= (int) $sw->id ?>="<?= e(number_format($d, 2, '.', '')) ?>">
-                                        <?php if ($taskStatusEnabled): ?>
-                                            <span class="assign-cell assign-status-<?= e($st) ?>"
-                                                  data-assign-cell
-                                                  data-sw-id="<?= (int) $sw->id ?>"
-                                                  data-status="<?= e($st) ?>">
-                                        <?php endif; ?>
                                         <?php if ($currentUser->isAdmin): ?>
                                             <input type="number" min="0" step="0.5"
                                                    value="<?= e(fmt_days($d)) ?>"
@@ -490,7 +490,6 @@ if (!function_exists('fmt_days')) {
                                                     <option value="<?= e($opt) ?>" <?= $opt === $st ? 'selected' : '' ?>><?= e($opt) ?></option>
                                                 <?php endforeach; ?>
                                             </select>
-                                            </span>
                                         <?php endif; ?>
                                     </td>
                                 <?php endforeach; ?>