mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 11:12:35 +02:00
fix(label-picker): allow spool list to shrink so all 4 templates and Cancel stay visible (issue #1230)
The Print Labels modal used a flex column with overflow-hidden on the outer container, the spool list as the flex-1 shrinkable child, and the templates + footer as fixed siblings below it. The spool list had min-h-[160px], which combined with the implicit min-height: auto on flex items meant it could not yield space when the modal was tight — templates and the Cancel button overflowed the modal's max-h-[90vh] and got clipped. Reproducible on Windows 11 + Brave at 1080p with browser chrome / DPI scaling reducing the effective viewport. Switching to min-h-0 both removes the explicit floor and overrides min-height: auto so flex shrinking actually works; the spool list now yields height to keep all four templates and the Cancel button visible on constrained viewports. Larger viewports behave identically since flex-1 still grows to fill. Adds a regression test that asserts all four template names + the Cancel button render in the DOM and pins the structural fix by checking the spool list scroller has min-h-0 with no min-h-[…] literal.
This commit is contained in:
@@ -14,6 +14,7 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
- **New AMS RFID rolls auto-named to the wrong colour when the hex is shared across material variants** ([#1227](https://github.com/maziggy/bambuddy/issues/1227)) — Inserting an Ivory White (PLA Matte) roll always created a spool named "Jade White" because the colour-catalog lookup in `create_spool_from_tray` filtered by manufacturer + hex only, with no `ORDER BY`. Three Bambu Lab catalog rows share `#FFFFFF` — Jade White (PLA Basic), Ivory White (PLA Matte), White (PLA Silk) — and SQLite returned them in rowid order, so the first-inserted entry (Jade White) won every time regardless of the actual material the AMS reported. Same class of bug bites any other shared-hex pair across PLA Basic / Matte / Silk; the whites were just the most visible. **Fix:** `spool_tag_matcher.py::create_spool_from_tray` now filters the catalog by `tray_sub_brands` too — the printer-reported material variant ("PLA Matte" / "PLA Basic" / "PLA Silk") matches the catalog's `material` column directly. The query also gets an explicit `ORDER BY id` so the fallback path (when `tray_sub_brands` is empty — third-party spools / OpenTag tags) is deterministic across SQLite + PostgreSQL instead of DB-implementation-defined. The catalog lookup uses the *raw* `tray_sub_brands` value (before the gradient/dual/tri-color subtype upgrade at lines 73-87) because the catalog stores `"PLA Basic"` for gradient rolls too — the upgraded subtype lives on the spool, not the catalog row. **Note for affected users:** spools already in the database under the wrong colour name (e.g. four Ivory White rolls labelled "Jade White") don't auto-correct on next AMS read — the matcher only fires when *creating* a new spool from RFID. Existing rows need a manual rename in Inventory after upgrading. **Tests:** 4 new in `test_spool_tag_matcher.py` — `test_ivory_white_pla_matte_resolves_to_ivory_not_jade` (the #1227 regression pin), `test_pla_silk_white_resolves_to_white_not_jade` (the third collision), `test_jade_white_pla_basic_still_resolves_correctly` (happy-path guard with all three #FFFFFF entries seeded), and `test_unknown_material_falls_back_to_hex_only_lookup` (third-party / empty `tray_sub_brands` path stays deterministic via ORDER BY).
|
||||
- **Backups to Gitea / Forgejo failed with "Failed to create tree" on empty repos and "list indices must be integers or slices, not str" on populated repos** ([#1224](https://github.com/maziggy/bambuddy/issues/1224), [#1225](https://github.com/maziggy/bambuddy/issues/1225)) — Two interacting bugs in the Gitea/Forgejo backend, both inherited from `GitHubBackend` because PR #1160's class docstring assumed Gitea's Git Data API was fully GitHub-compatible. (1) **List-shaped ref response:** `GET /api/v1/repos/{owner}/{repo}/git/refs/heads/{branch}` returns a *list* of matching refs on Gitea/Forgejo even when only one matches (`[{"ref": ..., "object": {"sha": ...}}]`), whereas GitHub returns a single object. The inherited `push_files` and `_create_branch_and_push` did `ref_response.json()["object"]["sha"]` and crashed with `list indices must be integers or slices, not str` — surfacing as the failure at the top of any push against a populated Gitea repo (#1225's symptom, and #1224's symptom once the user committed any file before the first backup). (2) **Empty-repo writes refused:** GitHub's Git Data API accepts `POST /git/blobs` against a brand-new empty repo and creates the initial commit + branch implicitly. Gitea refuses every blob/tree/commit POST with 404 until the underlying git repo has at least one commit — so the inherited `_create_initial_commit` (which posts blobs → tree → commit → ref in that order) silently failed: every blob POST returned 404, `tree_items` ended up empty, and the next tree POST also returned 404 ("Failed to create tree" — #1224's symptom on a freshly-created empty Gitea repo). **Fix:** `GiteaBackend` now overrides `push_files`, `_create_branch_and_push`, and `_create_initial_commit` directly instead of inheriting them. The Git Data API path uses a `_ref_sha()` helper that accepts both list and dict shapes; the empty-repo bootstrap route uses Gitea's Contents API (`POST /api/v1/repos/{owner}/{repo}/contents` with a `files` array, `branch=<target>`, `new_branch=<target>`) which seeds the initial commit + branch in a single transaction — Contents API is documented to work on empty repos because it goes through Gitea's higher-level repo-init path. `GitHubBackend` is **untouched** — the GitHub backup path is proven working, the fix is fully isolated to the Gitea side. `ForgejoBackend(GiteaBackend)` inherits both fixes automatically; tests pin that. **Tests:** 10 new tests in `test_git_providers.py` — `TestGiteaBackendListShapeRefResponse` (4 tests: `_ref_sha` accepts list/dict/empty-list, plus full `push_files` happy paths against list-shaped branch ref and list-shaped default-branch ref), `TestGiteaBackendEmptyRepoInitialCommit` (4 tests: empty repo routes through Contents API exclusively with no blob/tree/commit/ref Git Data API calls, payload shape verified field-by-field against Gitea's documented schema, error truncation works, empty file dict returns `skipped` without firing a useless API call), and `TestForgejoInheritsGiteaFixes` (2 tests: list-shape and empty-repo paths both work via inheritance). Existing 6 `TestGiteaBackendPushFiles` tests still pass since `_ref_sha` accepts dict-shaped responses too. Total: 78 tests pass across the backup unit + integration suites; ruff clean.
|
||||
- **Docker data-volume ownership normalised at startup via gosu entrypoint** ([#1211](https://github.com/maziggy/bambuddy/issues/1211)) — Two long-standing failure modes have been biting Docker users repeatedly: (1) Docker named volumes are created by the daemon as `root:root`, and the previous `chmod 777 /app/data` Dockerfile workaround only covered the named-volume root — so subdirs Bambuddy creates at runtime (`virtual_printer/uploads`, `virtual_printer/certs`, etc.) inherited wrong ownership when the container ran as `1000:1000`. (2) The shipped `docker-compose.yml` ships `./virtual_printer:/app/data/virtual_printer` uncommented, and dockerd creates a missing bind-mount source on the host as root before the container starts — leaving the host directory unwritable by uid 1000 inside the container even though the named volume above it had the chmod-777 workaround. Symptom either way: `[Errno 13] Permission denied: '/app/data/virtual_printer/uploads'`, no virtual printer ever starts, "VP doesn't work" support reports follow. **Replaces the chmod-777 hack with a proper entrypoint:** `deploy/docker-entrypoint.sh` runs as root, chowns `/app/data` and `/app/logs` (and `/app/data/virtual_printer` when bind-mounted) to `PUID:PGID`, then drops to that uid via `gosu` before `exec`'ing the app. The chown is gated behind a top-level ownership check so subsequent restarts skip the recursive traversal — no multi-second startup penalty on multi-GB archive directories. A sentinel `.bambuddy` file in each data path prevents Docker from re-syncing image directory metadata on every mount (otherwise empty volumes have their ownership reverted from the image on each restart, defeating the idempotency). When the container is started with an explicit `user:` directive or `--user` flag the entrypoint detects it isn't root and falls through to direct `exec` — preserving compatibility for users who pin a specific uid. **Compose template changes:** removes `user: "${PUID:-1000}:${PGID:-1000}"` (the entrypoint owns privilege drop now), adds `PUID` / `PGID` env vars with the same defaults, and comments out the `./virtual_printer:/app/data/virtual_printer` bind mount by default with explicit "only needed if you also run a native install of Bambuddy on the same host and want both to share the VP CA cert" guidance. The entrypoint chowns the host-side dir through the bind mount the first time it sees wrong ownership, so existing uncommented installs continue to work and #1211 specifically gets fixed.
|
||||
- **Label picker modal clipped the 4th template option and Cancel button on short viewports** ([#1230](https://github.com/maziggy/bambuddy/issues/1230), reported by @elit3ge) — Clicking "Print labels" from Inventory opened the picker with only 3 of the 4 templates visible (Avery 5160 was half-cut at the bottom) and no Cancel button reachable, with no way to scroll to them. Surfaced reliably on Windows 11 + Brave at 1080p with browser chrome / DPI scaling shrinking the effective viewport, but the layout bug hits anywhere the modal's `max-h-[90vh]` lands below ~770 px. **Cause:** `LabelTemplatePickerModal.tsx` uses a flex column with `overflow-hidden` on the outer modal, the spool list as the `flex-1` shrinkable child, and the templates section + footer as fixed siblings below it. The spool list had `min-h-[160px]`, which combined with the default `min-height: auto` for flex items meant the spool list couldn't yield space when the modal was tight — the templates and footer overflowed the modal's bottom edge and got clipped. **Fix:** `min-h-[160px]` → `min-h-0` on the spool list scroller, which both removes the fixed floor and overrides the implicit `min-height: auto` so flex shrinking actually works; the spool list now yields height to keep all four templates and the Cancel button visible on constrained viewports. On larger viewports the behaviour is identical (`flex-1` still grows to fill). Pre-existing on `dev` since 0.2.4b2 (commit `864e5c99`, the original PR #809 that introduced the modal); not a regression from the spoolman-inventory rebase. **Test:** new regression test in `LabelTemplatePickerModal.test.tsx` asserts all four template names + the Cancel button render in the DOM, and pins the structural fix by checking the spool list scroller has `min-h-0` and no `min-h-[…]` literal — so a future refactor that re-introduces a fixed floor on that element fails CI.
|
||||
|
||||
## [0.2.4b2] - 2026-05-05
|
||||
|
||||
|
||||
@@ -274,4 +274,37 @@ describe('LabelTemplatePickerModal', () => {
|
||||
fireEvent.change(screen.getByPlaceholderText(/Search/i), { target: { value: 'zzz-no-match' } });
|
||||
expect(screen.getByText(/No spools match/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('lets the spool list shrink (min-h-0) so all 4 templates and Cancel stay visible on short viewports (#1230)', () => {
|
||||
// Regression for #1230: on viewports where 90vh is tight (Windows 11
|
||||
// browser-chrome or DPI scaling), an explicit min-h on the spool list
|
||||
// pinned it taller than the modal could give back to templates + footer,
|
||||
// and `overflow-hidden` on the outer modal clipped the 4th template
|
||||
// (Avery 5160) and the Cancel button. The fix is `min-h-0` so the
|
||||
// flex-1 spool list can yield space when needed.
|
||||
const { container } = render(
|
||||
<LabelTemplatePickerModal
|
||||
isOpen={true}
|
||||
onClose={vi.fn()}
|
||||
availableSpools={SPOOLS}
|
||||
initialSelectedIds={[]}
|
||||
spoolmanMode={false}
|
||||
/>,
|
||||
);
|
||||
|
||||
// All 4 templates must be in the DOM, including the last one.
|
||||
expect(screen.getByText(/AMS holder/i)).toBeInTheDocument();
|
||||
expect(screen.getByText(/Box label/i)).toBeInTheDocument();
|
||||
expect(screen.getByText(/Avery L7160/i)).toBeInTheDocument();
|
||||
expect(screen.getByText(/Avery 5160/i)).toBeInTheDocument();
|
||||
expect(screen.getByRole('button', { name: /Cancel/i })).toBeInTheDocument();
|
||||
|
||||
// Structural guard against the regression: the scrollable spool list
|
||||
// must have `min-h-0` so flex shrinking actually works, and must NOT
|
||||
// pin a fixed minimum height that prevents it.
|
||||
const spoolListScroller = container.querySelector('div.flex-1.overflow-y-auto');
|
||||
expect(spoolListScroller).not.toBeNull();
|
||||
expect(spoolListScroller!.className).toContain('min-h-0');
|
||||
expect(spoolListScroller!.className).not.toMatch(/min-h-\[\d/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -308,7 +308,7 @@ export function LabelTemplatePickerModal({
|
||||
</div>
|
||||
|
||||
{/* Spool list */}
|
||||
<div className="flex-1 overflow-y-auto px-2 pb-2 min-h-[160px]">
|
||||
<div className="flex-1 overflow-y-auto px-2 pb-2 min-h-0">
|
||||
{visibleSpools.length === 0 ? (
|
||||
<div className="text-center text-sm text-bambu-gray py-6">
|
||||
{sortedSpools.length === 0
|
||||
|
||||
Reference in New Issue
Block a user