crueber/walhub

Watch

Fork 0

Releases UI: redesign list + detail pages to match the other tabs, and fix the asset-upload crash ("Cannot set properties of null (setting 'value')") #270

New issue

Closed

opened 2026-09-10 11:00:32 +00:00 by crueber · 3 comments

crueber commented

2026-09-10 11:00:32 +00:00

Owner

Three problems in the releases surface: the list and detail pages don't match the design language of the sibling tabs, and asset upload crashes with a JS error toast.

1. Asset upload crash (the bug — smallest, fix first)

Uploading an asset shows a toast: assetCannot set properties of null (setting 'value').

Root cause: web/src/pages/Release.jsx:70-85 onFile reads and then resets the file input after an await:

const onFile = async (e) => {
  const file = e.currentTarget.files?.[0];
  ...
  await ctx.repoClient.releases.uploadAsset(...);   // line 75
  e.currentTarget.value = "";                        // line 78 — BOOM

In SolidJS the event object's currentTarget is nulled/recycled once the handler yields — after the await, e.currentTarget is null, and setting .value throws TypeError, which the catch reports with the "asset" prefix → the exact toast text.

Fix: capture the input element in a let ref (let fileInput) and reset via fileInput.value = "" instead of the event object. Same class of bug may exist in other onChange handlers that touch e.currentTarget after an await — grep the pages for the pattern while in here.

2. Releases list page doesn't match the sibling tabs

web/src/pages/Releases.jsx renders a two-column layout (grid lg:grid-cols-[1fr_320px], :80) with a plain card-list of tag-name rows and a Latest-release sidebar. Compare with the tabs it should match:

Issues (Issues.jsx:62-77): page heading ("Issues", text-lg), filter row (state select, search input), toolbar buttons (Labels / Milestones / New issue) right-aligned with the primary CTA treatment, open/closed state chips.
The releases list has no page heading, no filter/search, its "New release" CTA is not primary-styled, and rows show only tag/name/date/asset-count — no body excerpt, no draft/prerelease filter, no pagination affordance beyond a bare "more" button.

Design direction (match the established language):

Page heading + toolbar row mirroring Issues: heading "Releases", right-aligned New release as btn primary, keep refresh.
Release rows: tag in mono + ReleaseBadges chips (exists), release name as the primary text (currently the reverse — tag dominates, name is muted meta), body excerpt (first line/plain-text of body), asset count, publish date via <DateTime>. Draft/prerelease filter chips (all / drafts / prereleases) client-side over the loaded page — no new endpoint.
Decide the Latest panel's fate: either keep the two-column grid but restyle the aside to match (it's close), or fold "Latest" into a pinned first card and go single-column like Issues. Either is defensible — pick one and apply consistently; the sidebar currently duplicates the first row's content on most repos.
Empty state (Empty component) already exists and is fine — keep.

3. Release detail page doesn't match either

web/src/pages/Release.jsx:100-148 renders tag + badges + rendered body in one card with a button row (edit / publish / delete / refresh) inline beneath the notes. Issues to align:

Heading hierarchy: h1 is the tag in mono; the release name is an h2 below. Compare Repo/Issue pages where the human title leads. Suggest: name as h1 (fallback to tag when unnamed — ReleaseNew already defaults name to tag), tag + badges + date as the meta row.
The action row mixes content-editing (edit), lifecycle (publish), destructive (delete), and navigation (refresh) in one undifferentiated row, left-aligned with ml-auto refresh. Match the issue detail's affordance placement (actions grouped top-right or under the header per the repo's established pattern) and use the danger treatment consistently for delete (it exists — btn danger — but sits mid-row).
The edit mode swaps the entire action row for name/textarea/save/cancel inline (:122-131) — restyle as a proper form section (the IssueNew/PullNew composer convention: centered max-w column, fieldsets, help text) rather than inputs floating in a button row.
Assets section (:150-173): plain list + a bare "upload asset" label-styled button. Give it a proper section header with count, asset rows with icon/name/size/sha/download/delete aligned like the rest of the app's table rows (data-table or the card-list conventions), and a clearer upload affordance (button + drop hint) with a busy state that reflects upload progress (currently the whole page's busy flag disables buttons but gives no progress indication for large files — client hashes via crypto.subtle before streaming, which can take seconds on big assets with zero feedback).

Acceptance criteria

Asset upload works: file uploads complete, the toast error is gone, the input resets, and the assets list refreshes (fix per §1; regression test or manual verify noted in PR).
Releases list: page heading + toolbar matching Issues; rows show name-led title, tag, badges, excerpt, asset count, date; draft/prerelease filter works; light/dark consistent.
Release detail: name-led heading, meta row (tag, badges, date, author), action placement matching sibling detail pages, edit mode as a proper composer form.
Assets section: section header with count, aligned rows, clear upload affordance with visible busy/progress state during hash+upload.
No new endpoints required — this is a web-only change (assets list already carries name/size/sha256).
Visual pass in light and dark themes; pnpm headless tests still green (update any snapshot/assertions the redesign touches).

Three problems in the releases surface: the list and detail pages don't match the design language of the sibling tabs, and asset upload crashes with a JS error toast. ## 1. Asset upload crash (the bug — smallest, fix first) Uploading an asset shows a toast: `assetCannot set properties of null (setting 'value')`. **Root cause:** `web/src/pages/Release.jsx:70-85` `onFile` reads and then resets the file input *after an await*: ```js const onFile = async (e) => { const file = e.currentTarget.files?.[0]; ... await ctx.repoClient.releases.uploadAsset(...); // line 75 e.currentTarget.value = ""; // line 78 — BOOM ``` In SolidJS the event object's `currentTarget` is nulled/recycled once the handler yields — after the `await`, `e.currentTarget` is `null`, and setting `.value` throws `TypeError`, which the `catch` reports with the `"asset"` prefix → the exact toast text. **Fix:** capture the input element in a `let` ref (`let fileInput`) and reset via `fileInput.value = ""` instead of the event object. Same class of bug may exist in other `onChange` handlers that touch `e.currentTarget` after an await — grep the pages for the pattern while in here. ## 2. Releases list page doesn't match the sibling tabs `web/src/pages/Releases.jsx` renders a two-column layout (`grid lg:grid-cols-[1fr_320px]`, :80) with a plain `card-list` of tag-name rows and a Latest-release sidebar. Compare with the tabs it should match: - **Issues** (`Issues.jsx:62-77`): page heading ("Issues", text-lg), filter row (state select, search input), toolbar buttons (Labels / Milestones / New issue) right-aligned with the `primary` CTA treatment, open/closed state chips. - The releases list has **no page heading**, no filter/search, its "New release" CTA is not `primary`-styled, and rows show only tag/name/date/asset-count — no body excerpt, no draft/prerelease filter, no pagination affordance beyond a bare "more" button. **Design direction (match the established language):** - Page heading + toolbar row mirroring Issues: heading "Releases", right-aligned `New release` as `btn primary`, keep refresh. - Release rows: tag in mono + `ReleaseBadges` chips (exists), release name as the primary text (currently the reverse — tag dominates, name is muted meta), body excerpt (first line/plain-text of `body`), asset count, publish date via `<DateTime>`. Draft/prerelease filter chips (all / drafts / prereleases) client-side over the loaded page — no new endpoint. - Decide the Latest panel's fate: either keep the two-column grid but restyle the aside to match (it's close), or fold "Latest" into a pinned first card and go single-column like Issues. Either is defensible — pick one and apply consistently; the sidebar currently duplicates the first row's content on most repos. - Empty state (`Empty` component) already exists and is fine — keep. ## 3. Release detail page doesn't match either `web/src/pages/Release.jsx:100-148` renders tag + badges + rendered body in one card with a button row (edit / publish / delete / refresh) inline beneath the notes. Issues to align: - Heading hierarchy: `h1` is the tag in mono; the release *name* is an `h2` below. Compare Repo/Issue pages where the human title leads. Suggest: name as `h1` (fallback to tag when unnamed — `ReleaseNew` already defaults name to tag), tag + badges + date as the meta row. - The action row mixes content-editing (edit), lifecycle (publish), destructive (delete), and navigation (refresh) in one undifferentiated row, left-aligned with `ml-auto` refresh. Match the issue detail's affordance placement (actions grouped top-right or under the header per the repo's established pattern) and use the `danger` treatment consistently for delete (it exists — `btn danger` — but sits mid-row). - The edit mode swaps the entire action row for name/textarea/save/cancel inline (:122-131) — restyle as a proper form section (the IssueNew/PullNew composer convention: centered max-w column, fieldsets, help text) rather than inputs floating in a button row. - Assets section (:150-173): plain list + a bare "upload asset" label-styled button. Give it a proper section header with count, asset rows with icon/name/size/sha/download/delete aligned like the rest of the app's table rows (`data-table` or the card-list conventions), and a clearer upload affordance (button + drop hint) with a busy state that reflects upload progress (currently the whole page's `busy` flag disables buttons but gives no progress indication for large files — client hashes via crypto.subtle before streaming, which can take seconds on big assets with zero feedback). ## Acceptance criteria - [ ] Asset upload works: file uploads complete, the toast error is gone, the input resets, and the assets list refreshes (fix per §1; regression test or manual verify noted in PR). - [ ] Releases list: page heading + toolbar matching Issues; rows show name-led title, tag, badges, excerpt, asset count, date; draft/prerelease filter works; light/dark consistent. - [ ] Release detail: name-led heading, meta row (tag, badges, date, author), action placement matching sibling detail pages, edit mode as a proper composer form. - [ ] Assets section: section header with count, aligned rows, clear upload affordance with visible busy/progress state during hash+upload. - [ ] No new endpoints required — this is a web-only change (assets list already carries name/size/sha256). - [ ] Visual pass in light and dark themes; `pnpm` headless tests still green (update any snapshot/assertions the redesign touches).

crueber referenced this issue from a commit

2026-09-10 11:23:34 +00:00

Fix #270: releases UI redesign + asset-upload crash

crueber referenced this issue from a pull request that will close it,

2026-09-10 11:23:39 +00:00

Fix #270: releases UI redesign + upload crash #279

crueber commented

2026-09-10 11:23:43 +00:00

Author

Owner

Fix is up: #279 (branch fix/issue-270, web-only, no new endpoints/deps). Crash fixed via captured input ref (+ page audit clean); list/detail redesigned per the issue with Latest folded into the first row. node --test 548/548 green, vite build clean. One gap: the shared CDP daemon here rejects target creation and tab attach, so the zero-console-error browser pass (list/detail/upload incl. crash scenario, both themes) still needs a reviewer with a working browser daemon — noted in the PR description.

Fix is up: #279 (branch `fix/issue-270`, web-only, no new endpoints/deps). Crash fixed via captured input ref (+ page audit clean); list/detail redesigned per the issue with Latest folded into the first row. `node --test` 548/548 green, `vite build` clean. One gap: the shared CDP daemon here rejects target creation and tab attach, so the zero-console-error browser pass (list/detail/upload incl. crash scenario, both themes) still needs a reviewer with a working browser daemon — noted in the PR description.

crueber referenced this issue from a commit

2026-09-10 11:52:12 +00:00

Fix #270 review: drop dead fileInput ref + doc wording

crueber commented

2026-09-10 11:52:22 +00:00

Author

Owner

Review of PR #279 (fix/issue-270, commit af27b50 + review fixup e40b31d):

CRASH FIX — correct. onFile captures the input synchronously (const input = e.currentTarget) and uploadFile never touches the event object; reset happens on the captured element in .finally with setUpload(null)/setBusy(false) in uploadFile's own finally (web/src/pages/Release.jsx:82-113). Audit of other async event handlers (IssueNew onPaste/onDrop extract files + preventDefault synchronously before await; Pull/ReleaseNew/Commit/Import/Keys only preventDefault before await) confirms the author's no-other-after-await claim — nothing missed.

LIST — matches Issues language: Releases h2 + right-aligned Refresh + primary New release (toolbar kept on the empty page too, superseding the #50 one-CTA rule — documented); All/Drafts/Prereleases pills (role=group, aria-pressed, counts) filter the loaded page client-side via filterReleases (no new endpoint); rows are name-led divider rows with tag mono, badges, excerptBody excerpt, asset count + DateTime; Latest folds into the matching first-page row only (filter==all, no cursor, tag match — no duplication with the old sidebar, which is deleted per the #35 supersede note); Older pagination; Empty kept for both empty and filtered-empty states.

DETAIL — name-led h1 with tag fallback, meta row (tag mono + badges + DateTime + author + short tag sha), regrouped actions (Edit / Publish primary when draft / Delete danger / Refresh), composer-section edit form (IssueNew/PullNew convention), assets as data-table (icon/name-link/size/short-sha + download/delete, overflow-x-auto) with staged Hashing…/Uploading… role=status affordance. Page pre-hashes via sha256Hex (web/sdk/src/releases.js) and passes the digest so the SDK skips its internal hash — verified SDK accepts {sha256}. No new deps, no new endpoints. Dark+light classes throughout (.pill/.chip/.btn danger/btn-active/data-table all exist in ui.css/base.css).

DOC — 12_web_ui.md REDESIGNED (#270) entry accurate, incl. #35/#50 supersede notes.

REVIEW FIXUP (pushed e40b31d): removed dead 'let fileInput' + ref={fileInput} — Solid ignores non-function refs so it never assigned; the event-capture is the real fix. Doc wording aligned.

VERIFY: node --test web/test/unit/*.test.js 548 pass / 0 fail; vite build clean (504 kB chunk-size warning only, pre-existing). Browser run explicitly skipped per instructions (node tests + reasoning only).

RECOMMENDATION: ready to merge.

Review of PR #279 (fix/issue-270, commit af27b50 + review fixup e40b31d): CRASH FIX — correct. onFile captures the input synchronously (const input = e.currentTarget) and uploadFile never touches the event object; reset happens on the captured element in .finally with setUpload(null)/setBusy(false) in uploadFile's own finally (web/src/pages/Release.jsx:82-113). Audit of other async event handlers (IssueNew onPaste/onDrop extract files + preventDefault synchronously before await; Pull/ReleaseNew/Commit/Import/Keys only preventDefault before await) confirms the author's no-other-after-await claim — nothing missed. LIST — matches Issues language: Releases h2 + right-aligned Refresh + primary New release (toolbar kept on the empty page too, superseding the #50 one-CTA rule — documented); All/Drafts/Prereleases pills (role=group, aria-pressed, counts) filter the loaded page client-side via filterReleases (no new endpoint); rows are name-led divider rows with tag mono, badges, excerptBody excerpt, asset count + DateTime; Latest folds into the matching first-page row only (filter==all, no cursor, tag match — no duplication with the old sidebar, which is deleted per the #35 supersede note); Older pagination; Empty kept for both empty and filtered-empty states. DETAIL — name-led h1 with tag fallback, meta row (tag mono + badges + DateTime + author + short tag sha), regrouped actions (Edit / Publish primary when draft / Delete danger / Refresh), composer-section edit form (IssueNew/PullNew convention), assets as data-table (icon/name-link/size/short-sha + download/delete, overflow-x-auto) with staged Hashing…/Uploading… role=status affordance. Page pre-hashes via sha256Hex (web/sdk/src/releases.js) and passes the digest so the SDK skips its internal hash — verified SDK accepts {sha256}. No new deps, no new endpoints. Dark+light classes throughout (.pill/.chip/.btn danger/btn-active/data-table all exist in ui.css/base.css). DOC — 12_web_ui.md REDESIGNED (#270) entry accurate, incl. #35/#50 supersede notes. REVIEW FIXUP (pushed e40b31d): removed dead 'let fileInput' + ref={fileInput} — Solid ignores non-function refs so it never assigned; the event-capture is the real fix. Doc wording aligned. VERIFY: node --test web/test/unit/*.test.js 548 pass / 0 fail; vite build clean (504 kB chunk-size warning only, pre-existing). Browser run explicitly skipped per instructions (node tests + reasoning only). RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-10 11:53:42 +00:00

crueber commented

2026-09-10 11:53:42 +00:00

Author

Owner

Fixed by PR #279 incl. review dead-ref fix (upload crash fixed + audit clean, Issues-parity list/detail; 548/548), merged. Closing.

crueber referenced this issue from a commit

2026-09-10 11:53:43 +00:00

Merge pull request 'Fix #270: releases UI redesign + upload crash' (#279) from fix/issue-270 into main

crueber referenced this issue from a commit

2026-09-10 13:32:38 +00:00