tighten up sgf download

[GreenAsJade]

Fixes players not realising they shouldn't download SGF during play, because that will tempt them to use assistance in game.

Proposed Changes

• Disable SGF download for players in game
• Disable SGF download for anonymous users in game

[github-actions]

Review

GameDock.tsx — Silent failure when clicking disabled SGF link

The old code showed an alert dialog when a user clicked the disabled download link:

void alert.fire(_("SGF downloading for this game is disabled until the game is complete."))

The new code swallows the click silently when disabled — the onClick handler just does nothing. Users (especially players who don't understand why the button is greyed out) will get no feedback. The tooltip title is still just "Download SGF", not an explanation of the restriction. This is a UX regression. Either keep an alert/toast on click, or update the tooltip title to explain the reason when disabled.

---

sgf-download-restrictions.ts — Fragile hardcoded waits

The test helpers use await page.waitForTimeout(500) to wait for the dock to reveal after hover, and the main test body uses await blackPage.waitForTimeout(1000) to wait for game-over state propagation. These are the exact antipatterns Playwright's docs warn against — they can produce flaky results in slow CI environments. The 500ms dock wait in particular is called multiple times per test run.

For the dock wait, consider waiting on the link itself becoming visible (e.g. await expect(link).toBeVisible()). For the game-over propagation, waiting for a DOM change that confirms the game ended (e.g. the GameDock re-rendering with the enabled state, or the game result element becoming visible) would be more reliable than a fixed 1-second sleep.

[GreenAsJade]

It seems dubious to me to fire an alert when someone clicks a disabled button. There are no other examples (that Claude could find) so I've left it off.

Test fixed.

[github-actions]

Two issues in GameDock.tsx:

1. Download SGF lost its href attribute

The original enabled Download SGF was a proper anchor:

<a href={sgf_url} target="_blank">

The new implementation replaces this with onClick + window.open() and no href. For a file download link this matters: right-click → "Save link as" / "Open in new tab" no longer works, and the URL no longer appears in the browser's status bar on hover. The same approach (no href, onClick only) is fine for navigation actions like "Review this game", but for a download link it's a regression.

A minimal fix is to keep href={sgf_url} target="\_blank" on the anchor and use ev.preventDefault() in the click handler when disabled.

2. No user feedback when clicking the disabled Download SGF button

Previously, when sgf_download_enabled was false the element had an onClick that fired an alert explaining why the download was unavailable. The new code silently ignores clicks on disabled elements. A player in an active game who clicks the greyed-out button gets no indication of why it doesn't work. At minimum a tooltip explaining the restriction (e.g. "SGF download is unavailable during an active game") would prevent confusion.

[GreenAsJade]

I think I already mentioned: no other element has an alert for clicking it when it is disabled.

I considered putting the info in the tooltip, but we generally dont even have tooltips, and these ones don't pop up if/when the dock opens, so they can't be relied on.

[github-actions]

Correction to previous automated review comments:

href is not missing — the earlier comment claiming "Download SGF lost its href attribute" was wrong. The diff clearly shows href={sgf_url} and target="_blank" are preserved on the new anchor.

Test timing — the earlier comment about hardcoded waitForTimeout calls does not apply to the current test. It uses proper Playwright patterns (waitFor, toBeVisible) throughout.

---

One genuine FYI: because href={sgf_url} is present on the disabled anchor, the browser context menu (right-click → "Open in new tab" / "Save link as") still works regardless of the disabled class. For UX discouragement this is probably fine, but if harder enforcement is ever needed it would require either removing href when disabled or a server-side check.

[GreenAsJade]

If someone save-ases the link, the backend change above blocks it.