Fix the findings from the RC9 test pass (#159-#163) - #165
Merged
Merged
Conversation
… you have Both goDownload and uploadStart opened the destination with os.Create on the final name. That truncates an existing file, so starting a transfer of an image already on the drive destroyed it before a single new byte arrived - and the cleanup on cancel, added for #153, then removed what was left. Pressing Download on an image you already had and changing your mind lost it, as did a dropped network or a board reset mid-transfer. The bytes now land in <name>.img.xz.part and are renamed into place only once they are all there, which also makes the replacement atomic. The suffix keeps partials out of getLocalImages' *.img.xz glob, so they never appear in the install list, and refreshProgress samples the partial because that is where the download is writing. TestDownloadCancelStopsTheTransfer watched the final name for its "the transfer has started" signal, which now only appears on completion; it watches the partial instead and additionally asserts that no image is left under the real name. Verified on A8 s/n 0498: with rebuild-barebone-v1.0.2.img.xz on the drive, starting the same download and cancelling left the original intact - same size, same mtime - and removed the .part. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All three were found in one pass over the UI on A8 s/n 0498, and all three live in App.vue. #160, the one that can cost you an image: the grid cell holding the select, the integrity icon and Delete is a fixed fraction of the row - 125px at a 960px viewport - while its contents are wider than that. With nowrap they overflowed the cell and were drawn on top of the Install button in the next one, so the two shared the same pixels and a click meant for Install could delete instead. Wrapping puts Delete on its own line. Measured at 860, 960, 1100 and full width: no overlap at any of them, and desktop widths are unchanged because everything still fits on one line. #161: getInfo() is fetched once per page load, which is right for the version, revision and serial number but not for emmc_version - a flash is precisely what changes it. The pipeline went on showing the image that was on the eMMC before, on the one screen meant to confirm the flash worked. The three paths that write the eMMC now share onFlashFinished(), so the refresh cannot be added to one and forgotten in the others. #162: onSelectedFileChanged told the integrity checker only when a file was selected, so clearing the selection - which is what deleting the selected image does - left the old verdict on screen. A green check next to "Please select one" reads as "the thing I am about to install is fine". fileSelected() already hides the icon for an empty name; it just has to be called. #160 and #161 were verified on the board. #162 has unit tests rather than a live check: after an install the drive is unmounted, so the image list is empty and there is nothing to select. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ut (#163) Reboot now and Shut down sit next to each other and fired on a single click, while Delete - which destroys far less - asks twice. The two outcomes are not equally cheap either: a stray Shut down on a headless board needs someone to walk over and power-cycle it. Both now use the same two-click pattern as Delete, including its reason for not using confirm(): a native dialog blocks the page. Arming one disarms the other, so a click meant for Reboot cannot confirm a Shut down armed moments earlier. The Recore Serial Number dialog offered only "Set Serial Number", so anyone who opened it to read the number had to know that clicking the backdrop dismisses it - and on the board's own touchscreen there is not much backdrop to aim at. Adding Cancel surfaced a second bug: the dialog never told the parent it had closed, so App's openSerialNumber stayed true after any dismissal, the false -> true watch never fired again, and the button stopped opening the dialog altogether. It now emits close whichever way it goes away. Verified on A8 s/n 0498: first click arms, clicking the other button disarms the first, both disarm after a few seconds; Cancel closes the dialog and the button reopens it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The remaining time is a projection off a clock, and it could run backwards: the figure was split into minutes and seconds without ever being clamped, and Math.floor(-4 % 60) is -4, so "0m:-4s" reached the screen. Two ways in - a start time in the future, because the board and the browser do not share a clock, and a progress figure at or past 100 while the estimate is still being recomputed. Clamped before the split, so neither part can carry the sign. The elapsed time gets the same treatment, for the same reason. Two readability fixes alongside it. The metrics popup was rgba(20,20,20,0.92), so the page behind showed through the plots - the REFLASH wordmark and the version line ran underneath the numbers being read. And the three figures under the bar are spread by justify-space-between, which leaves no space at all once the row is narrow: elapsed, rate and remaining ran together as "3.7 MB/s3m:13s". The .wrapper class they sit in had no styles at all; it has a gap now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The branch on isFinite and the explicit negative test were doing what Math.max(0, ...) does on its own. The pre-existing isNaN guard below still covers progress == 0, where the projection is Infinity and the subtraction comes out NaN - Math.max passes that through untouched. No behaviour change; the four tests around it are unchanged and still pass. 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
1 participant
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.
Five issues found driving the UI on A8 s/n 0498 against
v1.1.0-RC8-15-g01b35ac(the code released as v1.1.0-RC9).What is in here
#159 - silent data loss. Both the download and the upload path opened the destination with
os.Createon the final name, which truncates. Starting a transfer of an image the drive already held destroyed it before a single new byte arrived, and the cancel cleanup from #153 removed the remains. Transfers now write<name>.img.xz.partand rename into place only when complete, which also makes the replacement atomic.#160 - overlapping click targets. The install row's grid cell is 125px at a 960px viewport while its contents are wider, so Delete was drawn on top of Install and a click meant for one could hit the other. The cell wraps now.
#161 - stale eMMC version.
getInfo()is fetched once per page load, so the pipeline kept showing the pre-flash image on the screen meant to confirm the flash worked.#162 - stale integrity verdict. Deleting the selected image left its green check next to "Please select one".
#163 - polish. Two-click confirm on Reboot/Shut down; a Cancel button on the serial number dialog (which also fixes it refusing to reopen after any dismissal, since it never emitted
close); an opaque metrics popup; spacing between the progress figures; and the remaining time can no longer go negative.Verification
make testpasses: Go, bats, and 133 vitest tests (10 new).Checked on the board, not just in unit tests:
rebuild-barebone-v1.0.2.img.xzon the drive, started the same download and cancelled it. The original survived with its size and mtime unchanged, the.partwas cleaned up, and mid-transfer the drive held both the intact original and the growing partial.rebuild-barebone-v1.1.0-RC8overv1.0.2; the eMMC box updated with no reload.#162 has unit tests rather than a live check - after an install the drive is unmounted, so the image list is empty and there is nothing to select.
Not included: #164 (backup is compression-bound), which is waiting on a board with heat pads before the numbers are worth acting on.
🤖 Generated with Claude Code