mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 03:01:21 +02:00
fix(ui): drying popover footer no longer clipped by iOS Safari URL bar (#1669)
100vh and window.innerHeight report the layout viewport on iOS Safari, so the popover's Start Drying button rendered behind the bottom toolbar overlay. Switch the popover maxHeight to 100dvh and default computePopoverPosition's viewportHeight to visualViewport.height (with innerHeight fallback). The flip-above decision and the body-scroll fallback now both run against the actually-visible area, so the footer button stays reachable on iPhone.
This commit is contained in:
@@ -5,6 +5,8 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
## [0.2.5b1] - Unreleased
|
||||
|
||||
### Fixed
|
||||
- **AMS drying popover's "Start Drying" button is no longer hidden behind iOS Safari's bottom URL bar on iPhone (#1669, reported via in-app bug report, iPhone 17 Safari)** — Reporter could see the temperature / duration sliders and the "Rotate spool during drying" checkbox but couldn't reach the orange Start Drying button at the bottom of the popover — only a thin sliver of it was visible just above Safari's URL bar. **Root cause:** the popover sizes its `maxHeight` against CSS `100vh` (`PrintersPage.tsx:5443`) and positions itself using `window.innerHeight` (via `computePopoverPosition`, `popoverPosition.ts:53`). On iOS Safari both of those report the **layout** viewport — the full screen ignoring the bottom URL/toolbar overlay — not the visual viewport. The popover therefore extends *behind* Safari's bottom toolbar and the footer button gets clipped. Earlier iterations of the same surface (#1447 popover-off-bottom, #1458 footer-scroll-reachability) fixed desktop / normal-viewport cases but assumed `100vh` matched the visible viewport. **Fix:** two-line change. (a) `frontend/src/pages/PrintersPage.tsx:5443` switches `maxHeight: calc(100vh - …)` → `calc(100dvh - …)` so the dynamic viewport units shrink with iOS toolbars. (b) `frontend/src/utils/popoverPosition.ts:53` defaults `viewportHeight` from `window.visualViewport?.height ?? window.innerHeight` so the flip-above decision also uses the actually-visible area; the existing optional override still wins (tests keep their explicit viewport values). Result: when the iOS toolbar is up, either the popover flips above the trigger earlier (visualViewport too short for below-placement), or the body scrolls within a capped maxHeight and the `shrink-0` footer stays pinned to the visible bottom — the Start Drying button is reachable in both cases. **Tests:** 3 new in `popoverPosition.test.ts::computePopoverPosition (#1669)` — flip-above triggers when visualViewport.height (700) is shorter than innerHeight (800) and the trigger position would only overflow under the visual viewport; falls back to innerHeight when visualViewport is unavailable (older WebViews / jsdom); an explicit `viewportHeight` override still wins over a configured visualViewport.height (test-injection contract). 8 pre-existing tests stay green. dvh / svh browser support — Safari 15.4+, Chrome 108+, Firefox 101+ — comfortably covers iPhone 17 Safari and every supported desktop browser; no behavioural change on non-iOS.
|
||||
|
||||
- **Print queue `require_previous_success` no longer cascades indefinitely after a user-cancelled print (#1667, fully root-caused by @599w6c26tv-droid)** — Reporter on an A1 saw a single user-cancelled print block 18 downstream queue items over 3 days, all marked `skipped` with `Previous print failed or was aborted`. They captured the override log line proving Bambuddy correctly detects the cancellation (`Overriding status 'failed' -> 'cancelled' for printer 1 (print was stopped from queue by user)`) but the scheduler's gate ignored the override; they dumped the affected DB rows confirming the cascade pattern; and they reproduced from clean state in one cycle. Two distinct bugs in one function (`PrintScheduler._check_previous_success` in `services/print_scheduler.py`): **(a)** The lookback query `.in_(["completed", "failed", "skipped", "aborted"])` excluded `cancelled`, so a user cancellation was never found as the most-recent predecessor — the query walked past it to whatever real outcome existed before. **(b)** The same lookback INCLUDED `skipped`, so once one item got skipped (under any reason — bug-cascaded or genuinely failure-gated) it became the next item's "failed predecessor" and the cascade compounded. **Fix:** swap the lookback list to `["completed", "failed", "cancelled", "aborted"]` and broaden the success check to `prev_item.status in ("completed", "cancelled")`. A user cancellation is a deliberate action — treating it as neutral matches the user's intent ("I'm done with that one, move on"); `skipped` is excluded so the query always walks back to the most recent REAL print attempt and `failed` / `aborted` still gate as before. **Conservative recovery migration**: a one-shot pass in `core/database.py::run_migrations` resets only the skipped items whose immediate real predecessor (by `completed_at` desc, excluding the skipped-cascade itself) was `cancelled` — same fingerprint as the bug, narrow enough not to disturb skipped items whose true predecessor was a real `failed` / `aborted` print. Items match on `status='skipped' AND error_message='Previous print failed or was aborted'` and the predecessor check via correlated subquery; logged per-row at INFO so operators can audit the count after upgrade. Portable across SQLite and Postgres. Idempotent (post-reset rows no longer match). **Tests**: 10 new behaviour tests in `test_check_previous_success.py` pin every status/cascade combination — bug A (cancelled → True), bug B (skipped walked past), the reporter's exact failed→cancelled→skipped→skipped→pending cascade, regression guards on real failed / aborted still gating, edge cases (no-predecessor, only-skipped history, completed-then-failed). 7 new tests in `test_cancellation_cascade_recovery_migration.py` pin the migration — skipped-after-cancelled resets, skipped-after-failed stays, skipped-after-aborted stays, different-error-message untouched, reporter's multi-item cascade resets all, idempotent on re-run, per-printer isolation. All green; full scheduler + migration test suite stays green.
|
||||
|
||||
- **Firmware-update check no longer 403s against Bambu Lab's Cloudflare-gated download page (#1666, reported by @arekm, with the working bypass demonstrated)** — Reporter on a fresh install hit `Could not reach Bambu Lab's firmware download page...` when checking firmware for an A1 Mini, and surfaced the diagnostic: `curl -H 'User-Agent: Bambuddy/1.0' https://bambulab.com/en/support/firmware-download/all` returns `HTTP 403 cf-mitigated=challenge` — Cloudflare upped the bot-protection on `bambulab.com` to a JA3 / TLS-fingerprint challenge. Plain Python TLS handshakes (httpx, requests, urllib) don't match Chrome's ClientHello bytes, so CF rejects before the request reaches the app layer. The `Accept` / `Accept-Language` header workaround we shipped for #1350 was below-HTTP and no longer enough. Existing users with a `build_id.json` on disk from a previous successful fetch kept working until Bambu rebuilt the page (every few weeks); fresh installs and wiped data dirs hit the wall immediately — exactly the reporter's path. **Fix: use `curl_cffi` for the two `bambulab.com` fetches only.** New dependency added to `requirements.txt`; `firmware_check.py` lazy-initialises a `curl_cffi.requests.AsyncSession(impersonate="chrome", ...)` for the `bambulab.com` calls (the index page that carries the Next.js `buildId`, and the per-model `_next/data/{buildId}/.../{api_key}.json` endpoint). Smoke-tested end-to-end against the live page: returns 200 OK + valid `buildId`, vs the reporter's 403. **Compliance framing matters here**: per the Bambu-compliance email from 2026-05-12, Bambuddy committed to "no falsified client identity." `curl_cffi`'s Chrome impersonation only governs TLS handshake bytes — the **HTTP-layer User-Agent is overridden back to `Bambuddy/1.0 (+https://github.com/maziggy/bambuddy)`** via the session's `headers=` parameter. Defensible read: TLS fingerprint matches Chrome (necessary because Python's TLS is the signal CF gates on), but every application-layer identity remains honestly Bambuddy. A new test (`test_bambulab_curl_cffi_session_keeps_honest_user_agent`) pins this — a future refactor that drops the `headers=` override would silently revert to curl_cffi's Chrome-default UA and break the compliance commitment; the test fails on any non-Bambuddy UA in the session. **Soft dependency**: if `curl_cffi` fails to import (rare platforms, alpine without wheels, etc.), the service logs a one-time warning at startup and falls back to httpx; wiki-based version detection continues to work for the badge, only the in-app firmware download URL stops resolving. New test `test_bambulab_get_falls_back_to_httpx_when_curl_cffi_missing` pins the fallback path. The wiki path (`wiki.bambulab.com`) and the CDN download path (`public-cdn.bblmw.com`) stay on httpx — neither sits behind the same JA3 gate. Three existing tests (`test_build_id_is_persisted_to_disk`, `test_build_id_falls_back_to_disk_on_403`, `test_download_page_unreachable_flag_set_on_403_json`, `test_download_page_retries_once_when_buildid_stale`) updated to mock `_bambulab_get` instead of the raw httpx client — a tighter mock target that's stable across the curl_cffi / httpx switch.
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { describe, it, expect, afterEach } from 'vitest';
|
||||
import { computePopoverPosition } from '../../utils/popoverPosition';
|
||||
|
||||
/**
|
||||
@@ -114,3 +114,93 @@ describe('computePopoverPosition (#1447)', () => {
|
||||
expect(pos.top).toBe(middleTrigger.bottom + 12); // 332
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Tests for #1669: on iPhone Safari the bottom URL bar overlays the layout
|
||||
* viewport, so window.innerHeight reports more vertical space than is
|
||||
* actually visible. The popover's Start button rendered behind the toolbar.
|
||||
* The helper now defaults viewportHeight from visualViewport.height when
|
||||
* present so flip-above triggers against the real visible area.
|
||||
*/
|
||||
describe('computePopoverPosition (#1669, iOS Safari visualViewport)', () => {
|
||||
const originalVisualViewport = Object.getOwnPropertyDescriptor(window, 'visualViewport');
|
||||
const originalInnerHeight = Object.getOwnPropertyDescriptor(window, 'innerHeight');
|
||||
|
||||
afterEach(() => {
|
||||
if (originalVisualViewport) {
|
||||
Object.defineProperty(window, 'visualViewport', originalVisualViewport);
|
||||
} else {
|
||||
// jsdom didn't set it; remove anything we added so other tests see the
|
||||
// pristine state.
|
||||
// @ts-expect-error — deleting an optional property on window
|
||||
delete window.visualViewport;
|
||||
}
|
||||
if (originalInnerHeight) {
|
||||
Object.defineProperty(window, 'innerHeight', originalInnerHeight);
|
||||
}
|
||||
});
|
||||
|
||||
it('flips above when visualViewport is shorter than innerHeight (iOS toolbar visible)', () => {
|
||||
// Simulate the iPhone 17 Safari case: layout viewport says 800, but the
|
||||
// bottom URL bar overlay takes 100px so visualViewport reports 700.
|
||||
Object.defineProperty(window, 'innerHeight', { value: 800, configurable: true });
|
||||
Object.defineProperty(window, 'visualViewport', {
|
||||
value: { height: 700 },
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
// Trigger near the visual-viewport bottom: bottom=650 + gap 4 + height
|
||||
// 320 = 974 > 700-8. Without the fix (innerHeight=800), 974 > 800-8 is
|
||||
// also true so it would flip — fine. But subtract: 650+324=974 > 792 (yes)
|
||||
// — so flip happens with either. To prove visualViewport matters we need
|
||||
// a trigger that fits *under innerHeight* but overflows *under
|
||||
// visualViewport*: bottom=400, height=320, total=724. 724 < 800-8 = 792
|
||||
// (no flip with innerHeight), but 724 > 700-8 = 692 (flip with
|
||||
// visualViewport).
|
||||
const trigger = { top: 380, bottom: 400, left: 400, right: 440 };
|
||||
const pos = computePopoverPosition({
|
||||
triggerRect: trigger,
|
||||
popoverWidth: 240,
|
||||
estimatedHeight: 320,
|
||||
// Intentionally NO viewportHeight — exercise the default path.
|
||||
viewportWidth: 1024,
|
||||
});
|
||||
// Above placement: trigger.top - gap - height = 380 - 4 - 320 = 56.
|
||||
expect(pos.top).toBe(56);
|
||||
});
|
||||
|
||||
it('falls back to innerHeight when visualViewport is unavailable', () => {
|
||||
// Some older WebViews / jsdom configurations don't expose visualViewport.
|
||||
// @ts-expect-error — deleting an optional property on window
|
||||
delete window.visualViewport;
|
||||
Object.defineProperty(window, 'innerHeight', { value: 768, configurable: true });
|
||||
|
||||
// Trigger near the bottom should still flip above using innerHeight.
|
||||
const trigger = { top: 680, bottom: 700, left: 400, right: 440 };
|
||||
const pos = computePopoverPosition({
|
||||
triggerRect: trigger,
|
||||
popoverWidth: 240,
|
||||
estimatedHeight: 320,
|
||||
viewportWidth: 1024,
|
||||
});
|
||||
expect(pos.top).toBe(680 - 4 - 320); // 356 (trigger.top - gap - height)
|
||||
});
|
||||
|
||||
it('respects an explicit viewportHeight even when visualViewport is set', () => {
|
||||
// Tests pass viewportHeight explicitly; that override must still win.
|
||||
Object.defineProperty(window, 'visualViewport', {
|
||||
value: { height: 200 },
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
const pos = computePopoverPosition({
|
||||
triggerRect: { top: 300, bottom: 320, left: 400, right: 440 },
|
||||
popoverWidth: 240,
|
||||
estimatedHeight: 320,
|
||||
viewportHeight: 768,
|
||||
viewportWidth: 1024,
|
||||
});
|
||||
// 320 + 320 = 640 < 768 - 8, so no flip — uses the override, not the 200.
|
||||
expect(pos.top).toBe(324);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5439,8 +5439,10 @@ function PrinterCard({
|
||||
// margin). When the popover is taller than that space — short
|
||||
// viewport, landscape phone, zoomed-in — the body scrolls and
|
||||
// the footer stays pinned, so the Start button is always
|
||||
// reachable (#1458 / #1447 follow-up).
|
||||
maxHeight: `calc(100vh - ${dryingPopoverPos.top}px - 8px)`,
|
||||
// reachable (#1458 / #1447 follow-up). dvh (not vh) so iOS
|
||||
// Safari's bottom toolbar overlay doesn't clip the footer
|
||||
// (#1669, iPhone 17 Safari).
|
||||
maxHeight: `calc(100dvh - ${dryingPopoverPos.top}px - 8px)`,
|
||||
}}
|
||||
onClick={e => e.stopPropagation()}
|
||||
>
|
||||
|
||||
@@ -46,11 +46,22 @@ export interface ComputePopoverPositionOpts {
|
||||
* doesn't push the popover off-screen.
|
||||
*/
|
||||
export function computePopoverPosition(opts: ComputePopoverPositionOpts): PopoverPosition {
|
||||
// iOS Safari's bottom URL/toolbar overlay is excluded from window.innerHeight
|
||||
// but included in the layout viewport, so a popover anchored against
|
||||
// innerHeight gets its footer clipped behind the toolbar (#1669, iPhone 17
|
||||
// Safari). visualViewport reflects the actually-visible area when the
|
||||
// toolbar is up; fall back to innerHeight where it isn't available.
|
||||
const visualHeight =
|
||||
typeof window !== 'undefined' && window.visualViewport
|
||||
? window.visualViewport.height
|
||||
: typeof window !== 'undefined'
|
||||
? window.innerHeight
|
||||
: 0;
|
||||
const {
|
||||
triggerRect,
|
||||
popoverWidth,
|
||||
estimatedHeight,
|
||||
viewportHeight = window.innerHeight,
|
||||
viewportHeight = visualHeight,
|
||||
viewportWidth = window.innerWidth,
|
||||
margin = 8,
|
||||
gap = 4,
|
||||
|
||||
File diff suppressed because one or more lines are too long
+1
-1
@@ -26,7 +26,7 @@
|
||||
|
||||
<!-- Splash screens for iOS -->
|
||||
<link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
|
||||
<script type="module" crossorigin src="/assets/index-BCHcur4g.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-SS1wDI23.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-DgecYhis.css">
|
||||
</head>
|
||||
<body>
|
||||
|
||||
Reference in New Issue
Block a user