Page MenuHomePhabricator

Create an empty state for dashboards
ClosedPublic

Authored by lpriestley on May 27 2014, 10:15 PM.
Tags
None
Referenced Files
F14837570: D9312.diff
Fri, Jan 31, 10:06 AM
Unknown Object (File)
Tue, Jan 28, 2:06 PM
Unknown Object (File)
Wed, Jan 22, 2:51 PM
Unknown Object (File)
Mon, Jan 20, 1:15 AM
Unknown Object (File)
Fri, Jan 17, 11:18 PM
Unknown Object (File)
Fri, Jan 17, 7:52 PM
Unknown Object (File)
Sat, Jan 11, 3:01 PM
Unknown Object (File)
Sat, Jan 11, 12:16 PM
Subscribers

Details

Summary

Fixes T5177. Not sure if checking for panelPHIDs is right, but seemed like a better choice than adding a new property on dashboard.

Test Plan

Create dashboard with no panels. Go to view dashboard. "view" page should have a placeholder that directs user to Manage Dashboard

Diff Detail

Repository
rP Phabricator
Branch
dashboardemptystate
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 726
Build 726: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

lpriestley retitled this revision from to Create an empty state for dashboards.
lpriestley updated this object.
lpriestley edited the test plan for this revision. (Show Details)
lpriestley added a reviewer: epriestley.
epriestley edited edge metadata.

Two teeny tiny nitpicks.

src/applications/dashboard/controller/PhabricatorDashboardViewController.php
57–64

This is verrrry nitpicky, but technically these should be:

$this->getApplicationURI("x/y/");

...not:

$this->getApplicationURI()."x/y/";

There's not much practical difference, but the method does some path normalization and passing the path as an argument is generally more consistent.

75

This call to render() should be unnecessary.

(Long ago a lot of things needed to have render() called, so old code still has calls, but in modern code it is usually called implicitly by the rendering pipeline.)

This revision now requires changes to proceed.May 27 2014, 10:19 PM
lpriestley edited edge metadata.

addressing code review

epriestley edited edge metadata.
This revision is now accepted and ready to land.May 27 2014, 10:34 PM
epriestley updated this revision to Diff 22093.

Closed by commit rP92ccadaa42d2 (authored by @lpriestley, committed by @epriestley).