fix(support): review follow-ups for the ticket attachment field - #893
Merged
Conversation
SupportAttachmentField and FilePickerField carried a near line-identical body (48 px thumbnail, file name, edit icon, placeholder row). Both are now thin wrappers around FilePreviewField and keep their own label, so the KYC and support surfaces render exactly as before — proven by the unchanged goldens. With one call site left, Image.file gains the two things it lacked in both: an errorBuilder, so an unreadable temp file (iOS reclaims those) shows a placeholder instead of a grey box plus a logged exception, and cacheWidth, so a 48 px thumbnail no longer decodes at full resolution.
The matrix only ever pumped a state without an attachment, so the placeholder row was gated and the filled one — 48 px thumbnail, an Expanded file name and two icons, one of them a 48x48 tap target — was not. The second group runs the same device x text-scale sweep with a long file name and additionally asserts the close target stays tappable. Counter-probe: dropping the Expanded around the file name turns 35 of the 35 new cells red while the code still analyzes clean.
sendMessage decided on trimmed but posted the raw message, so leading and trailing whitespace reached support. Counter-probe: restoring the old expression turns 1 of the 20 cubit tests red.
…erclip Five of the goldens regenerated in #890 are mapped into the handbook and now show the attachment field and the paperclip button, while the prose still enumerated the old form fields. These descriptions are the alt text for readers who cannot see the screenshots, so the gap hits exactly the people who depend on them. The submitting state was checked against the golden rather than assumed: the attachment region is pixel-identical to the default state, so only the tags, the message field and the button change.
No golden rendered a selected file, so the thumbnail path this PR touches had no baseline at all — the one state where cacheWidth changes the decode resolution. Image.file decodes on the real async timeline and the shared precacheImages helper ends in pumpAndSettle, which does not return here. The test therefore polls for the decoded RawImage and asserts it, so a slower runner fails loudly instead of capturing an empty thumbnail as the new baseline. With the poll budget set to zero the test goes red. The baseline PNG was rendered by golden-regenerate.yaml on the self-hosted runner, under the same toolchain that validates it; it is carried here byte for byte so the branch keeps a fully signed history.
joshuakrueger-dfx
force-pushed
the
fix/support-attachment-followup
branch
from
August 4, 2026 15:25
f48fa55 to
3fd3f6e
Compare
TaprootFreak
added a commit
that referenced
this pull request
Aug 4, 2026
…asset smoke The two README mentions now use the canonical four-gate phrasing, the paths-filter comment admits the self-entry, and the smoke description states what the HTTP probes actually prove: per-file existence comes from the assembly count guards, the probes only see the auth gate answer (401 arrives before nginx looks any file up — tracked in #893).
Contributor
joshuakrueger-dfx
marked this pull request as ready for review
August 4, 2026 15:50
TaprootFreak
added a commit
that referenced
this pull request
Aug 4, 2026
…od deploys (#892) ## Summary Two handbook changes following up on #885/#889: ### Walkthrough layout (user feedback on the live page) The three images of the insider-unlock explainer rendered neither cleanly side by side nor stacked: the flex items are caption-driven wrapper divs, so two images filled the row and the third wrapped below. The block now shows the **instructions first**, then the three images in an equal-thirds grid with captions wrapping under their image; below 700px the walkthrough stacks as full-width rows (matching the page's existing narrow-viewport behaviour, verified against a neighbouring section). Desktop and mobile renders were verified in a local browser before pushing; the final CSS-cascade cleanup is render-identical (byte-equal screenshots). ### Deploys: staging → production, DEV retired Every push to `staging` now deploys directly to handbook.realunit.app (`:latest`, PRD secrets). The `deploy-dev` job, the `develop` trigger and the `:beta` tag are removed, and the `paths:` filter is dropped deliberately: every staging merge ships, which also re-stages the api-/web-sourced handbook content the filter could never observe. All prose that described the old staging→DEV/develop→PRD split (workflow comments, both READMEs, three spots inside the handbook page itself) now describes the single production lane, and the build-check comments state precisely what that check gates (screenshot-assembly, store-listing sync, legal-sync, image build + container smoke) versus what only the deploy run exercises (SSH/secrets/rollout). The retired dev-handbook instance keeps serving its last state until it is decommissioned on the infrastructure side (container + DNS) — intentionally out of scope for this repo. The four `DEPLOY_DEV_*` repo secrets become unreferenced with this PR and can be deleted afterwards. ## Erratum Commit `b571848f` references issue #893 in its message; the intended reference is **#894** (pre-existing: the build check's asset smoke accepts 401 as existence proof, which the auth gate makes meaningless — found during the review passes here, tracked separately). Amending a pushed commit is out per repo policy, hence this note.
The previous commit made sendMessage post the trimmed body, but submit() still hoisted the raw one — so the chat trimmed and the ticket did not. That split is worse than the state before: the two paths behaving alike was the reason the chat side was a readability note rather than a defect. canSubmit already gates on message.trim().isNotEmpty, so trimming what is sent changes no acceptance decision, only what support receives. Counter-probe: dropping .trim() again turns 1 of the 80 support cubit tests red.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Folge-PR zu #890. Behebt die fünf nicht-blockierenden Findings aus der Review dort
(#890 (review)).
Not symptom-driven: Kein Nutzerfall, keine Störung in Produktion. Auslöser ist die Review auf
#890 — ein Approve mit fünf ausdrücklich nicht-blockierenden Findings, die dort nicht mehr
liegenbleiben sollten. Jedes Finding wurde vor dem Bauen am gemergten Stand nachgeprüft; keines
beruht auf einer Weitergabe der Review-Formulierung.
Scale: Finding 1 trifft die Alt-Texte von fünf Handbuch-Bildern, die beim nächsten Deploy
ausgetauscht werden — die Fläche für alle, die die Screenshots nicht sehen können. Finding 2:
der befüllte Anhang-Zustand war in 0 der 35 Matrix-Zellen abgedeckt. Findings 3+4: zwei
Aufrufstellen von
Image.file, beide ohne Fehlerbehandlung. Finding 5: eine Zeile.Smaller fix considered: Zu 1 nur die
alt-Attribute statt Alt und Beschreibung — verworfen,die Beschreibungen zählen die Formularfelder auf und wären danach weiter falsch. Zu 2 eine
einzelne Zelle statt der vollen Matrix — verworfen, die bestehende Gruppe fährt die volle Matrix
und die Kosten sind identisch. Zu 3+4
errorBuilder/cacheWidthan beiden Stellen duplizierenstatt zusammenzuführen — verworfen, das ist genau das Auseinanderdriften, das den doppelten Fix
erst nötig gemacht hat. Zu 5 ist die eine Zeile der Fix.
Was drin ist
FilePreviewField(refactor):SupportAttachmentFieldundFilePickerFieldtrugen einenfast zeilengleichen Rumpf. Beide sind jetzt dünne Hüllen um ein gemeinsames Widget und behalten
ihr eigenes Label und ihre eigene Optik.
Image.fileexistiert dadurch einmal statt zweimalund bekommt dort einen
errorBuilder(sichtbarer Platzhalter plus Logzeile statt grauer Box) undcacheWidth, damit ein 48-px-Thumbnail nicht in voller Auflösung dekodiert wird.test): zweite Gruppe über dieselbe Geräte-×-Textskalen-Matrix,mit langem Dateinamen, plus Prüfung, dass das 48×48-Schliessziel tappbar bleibt.
fix):sendMessageentschied auftrimmed, verschickte aber denrohen
message— führende und nachlaufende Leerzeichen landeten beim Support.submit()imTicket-Pfad zieht mit: dort prüfte
canSubmitlängstmessage.trim().isNotEmpty, gesendet wurdetrotzdem der rohe Text. Genau dieses Gleichverhalten war TaprootFreaks Grund, den Chat-Befund
als Lesbarkeitsnotiz statt als Defekt zu führen — es wäre kaputtgegangen, hätte nur der Chat
getrimmt.
docs): Beschreibungen und Alt-Texte für 250/251/252/258/259 kennen jetzt dasAnhang-Feld und die Büroklammer.
Finding 3 hatte TaprootFreak ausdrücklich als „später" markiert; es ist auf Wunsch mit drin, weil
es Finding 4 auf eine einzige Stelle reduziert.
Verifikation
flutter analyzeohne Befund. 596 Tests grün übertest/widgets/form,test/screens/support,test/screens/settings_user_dataund die Goldens beider Flächen, dazu 19 Golden-Tests derSupport-Fläche inklusive des neuen.
Gegenproben, alle nachgefahren:
trimmed→messageim Chat zurückgedreht.trim()im Ticket-Pfad entferntExpandedum den Dateinamen entfernt (analysiert weiter sauber)cacheWidthauf4errorBuilderentferntDie Aussage zum Absende-Zustand im Handbuch ist am Golden geprüft, nicht angenommen: die
Anhang-Region ist pixelgleich zum Default-Zustand, geändert sind nur Tags, Nachrichtenfeld und
Schaltfläche.
Baselines
Die bestehenden Goldens sind bewusst nicht regeneriert: der Umbau soll die Optik nicht
verändern, und dass sie unverändert grün sind, ist genau der Beleg dafür.
Sie deckten allerdings nur den leeren Zustand ab — kein Golden im Repo rendert je eine
ausgewählte Datei, und genau dort ändert
cacheWidthdie Dekodier-Auflösung. Deshalb kommt einneues Golden dazu: das Ticket-Formular mit gesetztem Anhang
(
support_create_ticket_page_attached).Der Weg dorthin ist nicht trivial:
Image.filedekodiert auf der echten Async-Zeitachse, und dergemeinsame
precacheImages-Helper endet inpumpAndSettle, das hier nicht zurückkehrt. Einefeste Warteschleife wäre die schlechteste Lösung gewesen — sie hätte auf einem langsameren Runner
ein leeres Thumbnail als Baseline eingefroren, grün. Der Test pollt daher auf das dekodierte
RawImageund assertiert es; mit Poll-Budget 0 wird er rot. Ein langsamer Runner scheitert damitlaut, statt ein falsches Bild festzuschreiben.
Die Baseline selbst erzeugt
golden-regenerate.yamlauf dem self-hosted Runner, nicht mein Mac —sonst rendert sie unter einer anderen Toolchain als der, die sie auf PRs validiert. Bis dieser
Lauf durch ist, ist
Visual Regressionerwartbar rot.Finding 3 ist damit erledigt, Finding 4 an der verbliebenen Stelle behoben.