Page MenuHomePhabricator

Use the new "CurtainObjectRefList" UI element for subscribers
ClosedPublic

Authored by epriestley on Feb 4 2020, 8:29 PM.
Tags
None
Referenced Files
F14087732: D20967.id49961.diff
Sat, Nov 23, 11:32 PM
Unknown Object (File)
Thu, Nov 21, 6:35 PM
Unknown Object (File)
Wed, Nov 20, 5:19 AM
Unknown Object (File)
Sat, Nov 16, 1:39 AM
Unknown Object (File)
Tue, Nov 12, 9:46 AM
Unknown Object (File)
Tue, Nov 12, 6:42 AM
Unknown Object (File)
Tue, Nov 12, 6:06 AM
Unknown Object (File)
Fri, Nov 8, 4:43 AM
Subscribers

Details

Summary

Depends on D20966. Ref T13486. Curtains currently render subscribers in a plain text list, but the new ref list element is a good fit for this.

Also, improve the sorting and ordering behavior.

This makes the subscriber list take up a bit more space, but it should make it a lot easier to read at a glance.

Test Plan

Viewed object subscriber lists at varying limits and subscriber counts, saw sensible subscriber lists.

Diff Detail

Repository
rP Phabricator
Branch
verylong2
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 23784
Build 32708: Run Core Tests
Build 32707: arc lint + arc unit

Event Timeline

This revision was not accepted when it landed; it landed in state Needs Review.Feb 4 2020, 8:38 PM
This revision was automatically updated to reflect the committed changes.

@epriestley: That's an awesome change, but now the list of subscribers is way more prominent than the assignee. Wouldn't it make more sense to put assignee (which is more important information IMO) before the list of subscribers?
Just a suggestion! Thanks for your awesome work 👍

My theory at one point was that the lowest position is more visible (i.e., easier to locate visually -- since you know it's at the bottom, and the bottom of the curtain is visually distinct) than positions in the middle of the curtain (which don't have a well-known position), even if they're higher on screen. So that's why "Assigned/Authored" are currently at the bottom. However, feedback doesn't necessarily seem to align well with this theory (several other users have also suggested that they'd prefer this UI use "higher = more important" ordering).

I think I'm likely to try to apply a similar treatment to "Tags" and then maybe move "Assigned To" and "Authored By" to the topmost positions, directly under the actions.

There's a vague technical issue that I don't have an extensibility pattern I'm thrilled about for lists like this. Some of the elements are provided by modules from other applications (subscribers, tags); others are provided by the application itself (authored by). Today, each module assigns each curtain element a numerical sort order. This is sort of fragile; it also means modules can not override order or reorder elements. A possible fix is to make the order configurable so it doesn't really matter what modules do. I'm not thrilled about this for other reasons, but it's cleaner than any technical ordering pattern I've come up with.