Page MenuHomePhabricator

New NUX states for Conpherence
ClosedPublic

Authored by chad on Sep 16 2016, 4:14 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 20, 6:20 AM
Unknown Object (File)
Fri, Dec 20, 5:41 AM
Unknown Object (File)
Thu, Dec 19, 10:47 PM
Unknown Object (File)
Wed, Dec 11, 12:18 PM
Unknown Object (File)
Sat, Nov 30, 10:39 AM
Unknown Object (File)
Wed, Nov 27, 3:55 AM
Unknown Object (File)
Nov 23 2024, 11:47 AM
Unknown Object (File)
Nov 18 2024, 10:50 PM
Subscribers

Details

Summary

Roughly, if user isn't in any rooms, search for joinable ones. If no results, show big NUX banner.

Test Plan

Left all rooms, got fallback, joined room, left room. Create new instance, see new NUX. Set instance to public, visit Conpherence with and without public rooms.

Diff Detail

Repository
rP Phabricator
Branch
conph-nux (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 13749
Build 17754: Run Core Tests
Build 17753: arc lint + arc unit

Event Timeline

chad retitled this revision from to New NUX states for Conpherence.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.
  • cleaner public nux state
epriestley edited edge metadata.

This looks/works fine, but the empty state is always rendered, even if the user has joined a room -- and this makes it much more expensive to render, since we unconditionally load and render 10 threads. In my sandbox, this makes loading a /Z123 thread page (as a fully-active user who has already joined a bunch of rooms) about 15% slower.

I think we can fix this like this, by only building the expensive NUX if $this->thread is not set:

$results = array();
if (!$this->thread) {
  // If we aren't looking at a thread...
  $results = <all the query logic>;
}

if ($results) {
  // render "join a room NUX"
} else {
  // render "Welcome" NUX
}

Slightly simpler would probably be adding this at the top:

if ($this->thread) {
  // User is looking at a thread, so we definitely aren't going to render NUX.
  return null;
}

But I'm not sure if that causes any issues.

This revision now requires changes to proceed.Sep 16 2016, 6:51 PM
chad edited edge metadata.
  • add nux only if no threads
epriestley edited edge metadata.
This revision is now accepted and ready to land.Sep 16 2016, 10:08 PM
This revision was automatically updated to reflect the committed changes.