Page MenuHomePhabricator

Replace old hard-coded URI-based "changes saved" jank with new overgeneralized cookie-based "changes saved" jank
ClosedPublic

Authored by epriestley on Apr 18 2020, 10:24 PM.
Tags
None
Referenced Files
F13267947: D21144.diff
Wed, May 29, 4:26 AM
F13264441: D21144.id50354.diff
Mon, May 27, 5:06 PM
F13254625: D21144.diff
Sat, May 25, 3:33 AM
F13253422: D21144.diff
Sat, May 25, 2:39 AM
F13231576: D21144.diff
Tue, May 21, 12:34 AM
F13210078: D21144.id50341.diff
Fri, May 17, 3:50 AM
F13185940: D21144.diff
Sat, May 11, 3:19 AM
F13179319: D21144.diff
Wed, May 8, 9:06 PM
Subscribers
None

Details

Summary

Ref T13515. Settings currently has some highly specialized code for rendering "Changes saved." messages. The "saved" state is communicated across a redirect-after-POST by adding /saved/ to the end of the URI.

This isn't great. It needs a lot of moving pieces, including special accommodations in routing rules. It's user-visible. It has the wrong behavior if you reload the page or navigate directly to the "saved" URI.

Try this scheme, which is also pretty sketchy but seems like an upgrade on the balance:

  • Set a cookie on the redirect which identifies the form we just saved.
  • On page startup: if this cookie exists, save the value and clear it.
  • If the current page started with a cookie identifying the form on the page, treat the page as a "saved" page.

This supports passing a small amount of state across the redirect-after-POST flow, and when you reload the page it doesn't keep the message around. Applications don't need to coordinate it, either. Seems somewhat cleaner?

Test Plan

In Firefox, Safari, and Chrome: saved settings, saw a "Saved changes" banner without any URI junk. Reloaded page, saw banner vanish properly.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable