LibreKAT/Ocideck

Watch

Fork 0

fix: verberg de logokiezer op web in plaats van een pad te bewaren dat niet bestaat #525

Merged

brenno merged 2 commits from fix/logokiezer-platformpoort into main

2026-07-22 10:25:48 +00:00

brenno commented

2026-07-22 10:25:46 +00:00

Owner

Summary

The logo picker stored a path to a file on the user's machine in the theme profile. In a browser that path does not exist: FilePicker returns a blob: URL, so the setting appeared to work and was gone after a reload. The other pickers were gated behind supportsLocalProjectFolders in #148; this one was missed.

Closes #506

Changes

settings_dialog_colors.dart — the whole logo setting (picker, position, size) is behind supportsLocalProjectFolders. Position and size have no meaning without a logo, so they travel with it. Footer and closing-slide settings deliberately stay: they need no filesystem.
settings_dialog.dart — _pickLogo carries the same gate as an early return. Belt and braces on purpose: a silent, unrecoverable setting is worse than a missing button, and it makes the file that contains the FilePicker call name the flag itself, which is what makes it statically checkable by #505.
settings_dialog_smoke_test.dart — guards the running half: on a platform with a filesystem the setting is still there, and the footer was not dragged along by the gate. Verified by inverting the gate, which makes it fail.
CHANGELOG under Fixed.

What this PR does not do, and why

No regression test for the reported bug. It cannot exist here: kIsWeb is always false under flutter test, the flag arrives through a conditional import with no injection point, and there is no --platform chrome target. A widget test would sit green around the broken branch — worse than no test, because it buys confidence that is not there. That half is #505's job, statically.

Found while verifying — filed separately

image_carousel_picker_actions.dart:_browse() has the same defect (pickFiles without withData: true, reading .path), and its callers in _editor_field.dart and app_shell_main_layout.dart carry no platform gate either. Kept out of this PR because the right fix there is genuinely different: a logo cannot work on web, but the carousel plausibly should, via bytes and the existing mem: pipeline. Different decision, different issue.

Gates

make check — green (86.3% coverage, per-file floor 0 below budget).
make check-secrets / make sast — still red for the pre-existing reasons in #522 and #521.

Principles

Touches no format, storage, dependency or outgoing traffic. It does touch a promise, in the honest direction: the web build now shows fewer controls rather than controls that ignore you. The cost is that web visibly offers less than desktop; unexplained silence is worse than an absence.

## Summary The logo picker stored a path to a file on the user's machine in the theme profile. In a browser that path does not exist: `FilePicker` returns a `blob:` URL, so the setting appeared to work and was gone after a reload. The other pickers were gated behind `supportsLocalProjectFolders` in #148; this one was missed. Closes #506 ## Changes - `settings_dialog_colors.dart` — the whole logo setting (picker, position, size) is behind `supportsLocalProjectFolders`. Position and size have no meaning without a logo, so they travel with it. **Footer and closing-slide settings deliberately stay**: they need no filesystem. - `settings_dialog.dart` — `_pickLogo` carries the same gate as an early return. Belt and braces on purpose: a silent, unrecoverable setting is worse than a missing button, and it makes the file that contains the `FilePicker` call name the flag itself, which is what makes it statically checkable by #505. - `settings_dialog_smoke_test.dart` — guards the *running* half: on a platform with a filesystem the setting is still there, and the footer was not dragged along by the gate. Verified by inverting the gate, which makes it fail. - CHANGELOG under Fixed. ## What this PR does not do, and why **No regression test for the reported bug.** It cannot exist here: `kIsWeb` is always `false` under `flutter test`, the flag arrives through a conditional import with no injection point, and there is no `--platform chrome` target. A widget test would sit green around the broken branch — worse than no test, because it buys confidence that is not there. That half is #505's job, statically. ## Found while verifying — filed separately `image_carousel_picker_actions.dart:_browse()` has the same defect (`pickFiles` without `withData: true`, reading `.path`), and its callers in `_editor_field.dart` and `app_shell_main_layout.dart` carry no platform gate either. Kept out of this PR because the right fix there is genuinely different: a logo *cannot* work on web, but the carousel plausibly should, via bytes and the existing `mem:` pipeline. Different decision, different issue. ## Gates - `make check` — green (86.3% coverage, per-file floor 0 below budget). - `make check-secrets` / `make sast` — still red for the pre-existing reasons in #522 and #521. ## Principles Touches no format, storage, dependency or outgoing traffic. It does touch a promise, in the honest direction: the web build now shows fewer controls rather than controls that ignore you. The cost is that web visibly offers less than desktop; unexplained silence is worse than an absence.

brenno added 2 commits

2026-07-22 10:25:46 +00:00

fix(instellingen): verberg de logokiezer op web 09a40e6235

De logokiezer bewaarde het pad naar een bestand op de machine van de
gebruiker in het themaprofiel. In de browser geeft FilePicker daar een
blob:-URL terug: de instelling leek te lukken en was na herladen weg.

De poort staat op twee plekken. In _logoSectionChildren verdwijnt de hele
logo-instelling — de positie en de grootte hebben zonder logo geen
betekenis. In _pickLogo staat hij ook, want een stille onherstelbare
instelling is erger dan een ontbrekende knop; en zo noemt het bestand met
de FilePicker-aanroep de vlag zelf, wat het statisch toetsbaar maakt.

Footer en slotdia blijven staan: die hebben geen bestandssysteem nodig.

Closes #506

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

test(instellingen): bewaak de draaiende helft van de logo-poort

CI / Gate (Linux) · Format · Analyze · Coverage (pull_request) Has been cancelled

Details

CI / Test (macos-latest) (pull_request) Has been cancelled

Details

CI / Test (windows-latest) (pull_request) Has been cancelled

Details

CI / Web hardening (pull_request) Has been cancelled

Details

CI / Docs links (pull_request) Has been cancelled

Details

CI / Supply-chain (Trivy · advisory) (pull_request) Has been cancelled

Details

CI / Gate (Linux) · Format · Analyze · Coverage (push) Has been cancelled

Details

CI / Test (macos-latest) (push) Has been cancelled

Details

CI / Test (windows-latest) (push) Has been cancelled

Details

CI / Web hardening (push) Has been cancelled

Details

CI / Docs links (push) Has been cancelled

Details

CI / Supply-chain (Trivy · advisory) (push) Has been cancelled

Details

c2416fc5d4

Deze test dekt niet de gemelde bug maar de terugslag ervan: dat de
logo-instelling op een platform mét bestandssysteem gewoon blijft staan,
en dat de footer niet is meegesleept door de poort. Geverifieerd door de
poort om te draaien — dan valt hij.

De web-helft is hier structureel niet te toetsen: kIsWeb is onder
flutter test altijd false, de vlag komt via een conditionele import
zonder injectiepunt, en er is geen --platform chrome-doel. Die kant
wordt door #505 bewaakt, statisch in plaats van met een test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

brenno merged commit 723a8d8138 into main

2026-07-22 10:25:48 +00:00

brenno referenced this pull request from a commit

2026-07-22 10:25:48 +00:00

Merge pull request 'fix: verberg de logokiezer op web in plaats van een pad te bewaren dat niet bestaat' (#525) from fix/logokiezer-platformpoort into main

brenno deleted branch fix/logokiezer-platformpoort

2026-07-22 10:25:48 +00:00