Details
- Reviewers
epriestley - Maniphest Tasks
- T7573: Ship Quicksand
- Commits
- Restricted Diffusion Commit
rPdd9ec255ecac: Quicksand - make notification and message counts update as you navigate around
loaded up two tabs, one with a durable column on and one without. in the without browser, i read some messages, decrementing my unread count. when i navigated again in the durable column browser, the count updated correctly. with no notifications, commented on a task with another user to get a notification and it showed up properly. visited the task by clicking not the notification and the bubble count decremented correctly
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Possible inline issue / possible fix? Maybe I'm misreading or misremembering.
| webroot/rsrc/externals/javelin/lib/Quicksand.js | ||
|---|---|---|
| 147 | I think this is wrong or misleading. This won't get set when you press "back", only when you navigate forward after pressing back. So I think events with "backNav" set are actually forward navigation events following back-navigation events. | |
| 195 | It might be cleaner to pass some fresh (or whatever) flag here, and then pass isFresh on the event, to indicate that the data is fresh from the server. | |
In particular, even if backNav is accurate, we don't want to update when the user presses "Forward" after pressing "Back" -- only when we get fresh new data from the server.
Cool, the rest of this looks good to me.
This is a sort of interesting example of T7447, too -- the ghosts are detached from their original context, and there's pretty much nowhere we could put them to make them work better, but it doesn't feel too confusing to me. However, this has like 4 comments and they were all written in the last 10 minutes so they're fresh in my memory, I'm not sure if it's good enough to do well in that easy case.