Fix/pet card layout and smoke flake - #139
Merged
Merged
Conversation
The pet card's detail grid was two columns on desktop, so each value got about 150px - not enough for a 12-character breed such as スコティッシュフォールド, which wrapped. Breed is free text, so a fixed column width fails again as soon as a longer name is entered. Lay each item out as one "label | value" row instead. Measured on the real page at 1280px (920px shell, three columns, 283px cards) the value column goes from 129px to 173px, and values line up across rows. Row spacing drops from 10px to 6px so the extra separators do not make the card taller: the card grows 16px on desktop and shrinks 120px on mobile. Replace system-ui with an explicit Japanese font stack as well. system-ui resolves to Yu Gothic UI on Windows, whose kana advance is about 0.72em, so the same text wrapped differently there than on macOS/Linux, where it is 1em: スコティッシュフォールド measured 139px against 204px. Naming the fonts narrows that gap - it is an approximation, not a guarantee, since it falls back to sans-serif where none are installed. Swept eight pages at 390px and 1280px: no horizontal scroll, nothing spilling out of the shell. The new E2E test fixes the width split - the value keeps room for at least ten full-width characters, on the same line as its label - rather than a character count, so it does not depend on which fonts the runner has. It fails both on the previous layout and on a two-column card grid. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"readiness waits through 500 until the app returns 200" failed intermittently with "Timed out waiting for http://smoke.invalid (last status: 500)". wait_until_available polls against a wall-clock deadline and slept 0.5s between attempts. The test queues five 500s before the 200, so 2.5s of the 5s budget went to sleeping, and spawning the stubbed curl six times cost another ~1.5s on Windows. That left about a second of headroom, which a busy machine ate. The suite already states at the top that nothing in it waits on real time - it zeroes SMOKE_RETRY_DELAY_SECONDS - but readiness had no such knob. Add SMOKE_READY_POLL_SECONDS (default 0.5, so CD is unchanged) and zero it in the tests, which cuts the readiness phase from ~4.0s to ~1.46s. Raise the test's --timeout from 5 to 20 as well, because the remaining cost is process startup, which cannot be zeroed: nothing here asserts a readiness timeout and the loop returns as soon as the queue yields 200, so the larger budget costs no run time. Guard the new default the way the retry delay is already guarded, since the tests force it to 0. Verified with the suite run five times standalone and three times while a dotnet test run loaded the machine: 8/8 each time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Two independent fixes, one per commit, touching disjoint files so either can be
reverted on its own.
1.
fix: give the breed value the card width in the pet list(42aaf66)On desktop the pet card laid its detail grid out in two columns, leaving each
value about 150px — not enough for a 12-character breed such as
スコティッシュフォールド, which wrapped mid-name. Breed is free text, so any
fixed column width fails again as soon as a longer name is entered.
Each item is now a single
label | valuerow. Measured on the real page at1280px (920px shell, three columns, 283px cards):
Row spacing drops from 10px to 6px so the doubled number of separators does not
inflate the card; values also line up across rows and across cards.
This commit also replaces
system-uiwith an explicit Japanese font stack.system-uiresolves to Yu Gothic UI on Windows, whose kana advance is about0.72em, so identical text wrapped differently there than on macOS/Linux, where
it is 1em — スコティッシュフォールド measured 139px against 204px. Naming the
fonts narrows that gap (Windows now measures 192px). It is an approximation,
not a guarantee: where none of the named fonts exist it still falls back to
sans-serif.Accepted trade-off
At three columns the value column is 173px, i.e. room for about ten full-width
characters. Longer breeds still wrap — now consistently on every OS rather than
only on some. Fitting twelve characters would need cards of 312px or more, which
means either a wider shell or dropping to two columns; three-column density was
chosen instead. The reasoning is recorded in the CSS.
Test
PetList_GivesBreedValueMostOfCardWidth_AcrossViewportsfixes the width split —the value keeps room for at least ten full-width characters, on the same line as
its label — rather than a character count, so it does not depend on which fonts
the CI runner has installed. It was confirmed to fail both on the previous
layout and on a two-column card grid.
2.
fix: stop the local-smoke readiness test flaking under load(133b7c3)readiness waits through 500 until the app returns 200failed intermittentlywith
Timed out waiting for http://smoke.invalid (last status: 500).wait_until_availablepolls against a wall-clock deadline and slept 0.5sbetween attempts. The test queues five 500s before the 200, so 2.5s of the 5s
budget went to sleeping, and spawning the stubbed curl six times cost another
~1.5s on Windows — about 1s of headroom, which a busy machine ate.
The suite already states at the top that nothing in it waits on real time (it
zeroes
SMOKE_RETRY_DELAY_SECONDS), but readiness had no such knob. This addsSMOKE_READY_POLL_SECONDS, defaulting to 0.5 so CD behaviour is unchanged, andzeroes it in the tests: the readiness phase drops from ~4.0s to ~1.46s. The
test's
--timeoutalso goes from 5 to 20, because the remaining cost is processstartup and cannot be zeroed; nothing here asserts a readiness timeout and the
loop returns as soon as the queue yields 200, so the larger budget costs no run
time. A static guard for the new default is added alongside the existing one for
the retry delay.
Test plan
scripts/build.ps1— 0 warnings, 0 errorsscripts/test.ps1— 240 unit tests, 8 shell testsscripts/format.ps1— cleannothing spilling out of the shell
dotnet testrun loaded the machine: 8/8 every time
Review notes
Per AGENTS.md this touches none of the areas that require human sign-off
(authorisation, existence hiding, returnUrl, image handling, deletion). The
changes are CSS, a shell script default, and tests.