Page MenuHomePhabricator

Add a Quicksand-aware page-level container to diffs
ClosedPublic

Authored by epriestley on May 8 2017, 4:01 PM.
Tags
None
Referenced Files
F14087501: D17840.id42917.diff
Sat, Nov 23, 10:06 PM
Unknown Object (File)
Tue, Nov 19, 9:23 PM
Unknown Object (File)
Sat, Nov 16, 9:21 AM
Unknown Object (File)
Tue, Nov 12, 2:45 PM
Unknown Object (File)
Fri, Nov 8, 2:07 PM
Unknown Object (File)
Fri, Nov 8, 6:29 AM
Unknown Object (File)
Sat, Oct 26, 5:10 PM
Unknown Object (File)
Oct 23 2024, 12:07 PM
Subscribers
None

Details

Summary

Ref T12616. Ref T8047. Ref T11093. We currently have several bugs where diff state sticks across Quicksand pages.

Add a new top-level object to handle this, with sleep() and wake() methods. In sleep(), future changes will remove/deacivate all the reticles/editors/etc.

See T12616 for high-level discussion of plans here.

This general idea is likely to become more formal eventually (e.g. for "sheets" or whatever we call them, in T10469) but I think this is probably a reasonable place to draw a line for now.

Test Plan
  • Added some logging to sleep(), wake() and construct().
  • Viewed changes in Differential.
  • With Quicksand on, browsed around; saw state change logs fire properly.

Diff Detail

Repository
rP Phabricator
Branch
inline1
Lint
Lint Warnings
SeverityLocationCodeMessage
Warningwebroot/rsrc/js/application/diff/DiffChangesetList.js:1JAVELIN5`javelinsymbols` Not In Path
Unit
No Test Coverage
Build Status
Buildable 16844
Build 22482: Run Core Tests
Build 22481: arc lint + arc unit

Event Timeline

  • Slightly less debugging code.
  • Make the state change more robust to Quicksand and behaviors firing in any order.
chad added inline comments.
webroot/rsrc/js/application/diff/DiffChangesetList.js
8–26

spacing expected?

This revision is now accepted and ready to land.May 8 2017, 10:08 PM
This revision was automatically updated to reflect the committed changes.