Conpherence - Differentiate audience of Threads/Rooms with icon
ClosedPublic
Actions

Authored by btrahan on Mar 25 2015, 10:24 PM.

Details

Reviewers

epriestley
chad

Maniphest Tasks

T7629: Differentiate audience of Threads/Rooms with icon

Commits

Restricted Diffusion Commit
rPe4b7263bf8dc: Conpherence - Differentiate audience of Threads/Rooms with icon

Summary

Fixes T7629 plus an un filed bug that's breaking creating new threads since we need to add participants EVEN EARLIER than we were doing it now that policy is actually enforced.

Back to the main thrust of this, there is one UI corner case - in the main view if you go from 1:1 to 1:1:1 (i.e. add a 3rd recipient, or Nth in a row) the icon only updates on page reload. I figure this will get sorted out at a later refactor as we make the client better / share more code with durable column.

One other small behavioral oddity is in the main view sometime we start loading with no conpherence. in that case, rather than show some incorrect icon, we show no icon (and "no title") and then things change at load. Seems okay-ish.

Finally, @chad - the CSS is a very work-man-like "use the built in stuff you can specify from PHP" so I'm sure it needs some love.

Test Plan

made all sorts of rooms and threads and liked the icons. noted smooth loading action as i switched around

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

btrahan updated this revision to Diff 29236.Mar 25 2015, 10:24 PM

btrahan added a task: T7629: Differentiate audience of Threads/Rooms with icon.

btrahan retitled this revision from to Conpherence - Differentiate audience of Threads/Rooms with icon.

btrahan updated this object.

btrahan edited the test plan for this revision. (Show Details)

btrahan added reviewers: epriestley, chad.

btrahan added a subscriber: chad.

Herald added subscribers: epriestley, Korvin. · View Herald TranscriptMar 25 2015, 10:24 PM

btrahan updated this object.Mar 25 2015, 10:26 PM

btrahan updated this object.

Screen_Shot_2015-03-25_at_3.36.33_PM.png (1×2 px, 591 KB)

Screen_Shot_2015-03-25_at_3.36.17_PM.png (1×2 px, 695 KB)

Maybe write a test for sending a new thread and creating a new room? I feel like that's broken a couple of times for similar policy-related reasons.

src/applications/conpherence/editor/ConpherenceEditor.php
224	Not sure if this is a bug, but: I would expect adding one participant to set the recent PHIDs to just them; this might wipe out older PHIDs? For example, recents are `(B, A)`. You add C and it becomes `(C)` but we'd expect it to become `(C, B, A)` or something like that?
244	"tne" => "the"