Skip to content

Commit 2aaadc7

Browse files
committed
Mark as read was reloading the whole page
1 parent e9fd85e commit 2aaadc7

4 files changed

Lines changed: 56 additions & 6 deletions

File tree

internal/web/static/app.js

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@
4444
credentials: 'same-origin'
4545
}).then(function (response) {
4646
if (!response.ok) {
47-
throw new Error('unexpected status ' + response.status);
47+
showArchiveError(form, button);
48+
return;
4849
}
4950
button.disabled = false;
5051
// Every control for this article, not just the one clicked: the reader
@@ -56,13 +57,27 @@
5657
toggle(other, other.querySelector('button'));
5758
}
5859
});
59-
}).catch(function () {
60-
// Fall back to the ordinary post rather than leaving the reader unsure
61-
// whether the click registered. Worst case they get the old behaviour.
62-
form.submit();
60+
}, function () {
61+
showArchiveError(form, button);
6362
});
6463
});
6564

65+
// Once the script has intercepted a submission it must never submit the same
66+
// form again: the request may have reached the server even when its response
67+
// was lost. Retrying automatically could therefore perform the action twice
68+
// and, more visibly, bring back the full-page reload this enhancement avoids.
69+
function showArchiveError(form, button) {
70+
button.disabled = false;
71+
var error = form.querySelector('.action-error');
72+
if (!error) {
73+
error = document.createElement('span');
74+
error.className = 'action-error';
75+
error.setAttribute('role', 'alert');
76+
form.appendChild(error);
77+
}
78+
error.textContent = 'Could not update. Try again.';
79+
}
80+
6681
// articleOf reads the article's id out of a form's address, which is either
6782
// /article/{id}/archive or /article/{id}/unarchive.
6883
function articleOf(form) {

internal/web/static/style.css

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,14 @@ a { color: var(--accent); }
159159
used to stand a few pixels taller than the links beside it. */
160160
form.inline { display: flex; margin: 0; }
161161

162+
.action-error {
163+
align-self: center;
164+
margin-left: 0.5rem;
165+
color: #a52f2f;
166+
font-family: system-ui, sans-serif;
167+
font-size: 0.75rem;
168+
}
169+
162170
.read-marker {
163171
margin: 0 0 1rem;
164172
font-family: system-ui, sans-serif;

internal/web/web.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,11 +244,21 @@ func (s *Server) Handler() http.Handler {
244244
// or a prefetching browser must not be able to empty the reading list.
245245
mux.HandleFunc("POST /article/{id}/{action}", s.handleArchive)
246246

247-
mux.Handle("GET /static/", http.FileServerFS(assets))
247+
mux.Handle("GET /static/", noCache(http.FileServerFS(assets)))
248248

249249
return s.setupGate(mux)
250250
}
251251

252+
// Embedded assets have no useful modification time for HTTP revalidation.
253+
// Asking browsers to revalidate prevents an old app.js from surviving an image
254+
// upgrade and silently losing newer progressive enhancements.
255+
func noCache(next http.Handler) http.Handler {
256+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
257+
w.Header().Set("Cache-Control", "no-cache")
258+
next.ServeHTTP(w, r)
259+
})
260+
}
261+
252262
// thousands groups a number with thin spaces, so seven figures can be read
253263
// without counting digits. A space rather than a comma or a full stop: the
254264
// reader is Italian and English, and those two disagree about which of them

internal/web/web_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -903,6 +903,23 @@ func TestScriptIsLoadedAndDeferred(t *testing.T) {
903903
}
904904
}
905905

906+
func TestStaticAssetsAreRevalidatedAfterAnUpgrade(t *testing.T) {
907+
handler := newTestServer(t, &fakeStore{})
908+
req := httptest.NewRequest(http.MethodGet, "/static/app.js", nil)
909+
rec := httptest.NewRecorder()
910+
handler.ServeHTTP(rec, req)
911+
912+
if rec.Code != http.StatusOK {
913+
t.Fatalf("status = %d, want 200", rec.Code)
914+
}
915+
if got := rec.Header().Get("Cache-Control"); got != "no-cache" {
916+
t.Errorf("Cache-Control = %q, want no-cache", got)
917+
}
918+
if got := strings.Count(rec.Body.String(), "form.submit()"); got != 1 {
919+
t.Errorf("form.submit() count = %d, want only the date-picker submission", got)
920+
}
921+
}
922+
906923
// The reader repeats both controls at the head of the page, so a long article
907924
// need not be scrolled to its end to be marked read or opened at source.
908925
func TestReaderRepeatsActionsAtTop(t *testing.T) {

0 commit comments

Comments
 (0)