Paths

Table of Contentst

Dashboards - add layout mode to dashboards
ClosedPublic
Actions

Authored by btrahan on May 9 2014, 11:50 PM.

Details

Reviewers

epriestley
chad

Maniphest Tasks

T4996: Implement new dashboard create flow

Commits

Restricted Diffusion Commit
rP630095566174: Dashboards - add layout mode to dashboards

Summary

This gets us the ability to specify a "layout mode" and which column a panel should appear in at panel add time. Changing the layout mode from a multi column view to a single column view or vice versa will reset all panels to the left most column.

You can also drag and drop where columns appear via the "arrange" mode.

We also have a new dashboard create flow. Create dashboard -> arrange mode. (As opposed to view mode.) This could all possibly use massaging.

Fixes T4996.

Test Plan

made a dashboard with panels in multiple columns. verified correct widths for various layout modes

re-arranged collumns like whoa.

Diff Detail

Repository

rP Phabricator

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

btrahan updated this revision to Diff 21452.May 9 2014, 11:50 PM

btrahan added a task: T4996: Implement new dashboard create flow.

btrahan retitled this revision from to Dashboards WIP - add layout mode to dashboards.

btrahan updated this object.

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

btrahan added reviewers: epriestley, chad.

Herald added subscribers: Korvin, epriestley. · View Herald TranscriptMay 9 2014, 11:50 PM

Harbormaster completed remote builds in B332: Diff 21452.May 9 2014, 11:52 PM

Screen_Shot_2014-05-09_at_4.51.35_PM.png (1×1 px, 507 KB)

Screen_Shot_2014-05-09_at_4.51.40_PM.png (1×1 px, 470 KB)

Screen_Shot_2014-05-09_at_4.51.46_PM.png (1×1 px, 471 KB)

Screen_Shot_2014-05-09_at_4.51.54_PM.png (1×1 px, 585 KB)

btrahan added inline comments.May 10 2014, 12:00 AM

src/applications/dashboard/controller/PhabricatorDashboardAddPanelController.php
71–76	do you think this should be its own transaction, like PhabricatorDashboardTransaction::TYPE_PANEL_COLUMN ? I kind of wanted to tuck it into the edge relationship, but that seemed hard to do / ill advised because the ApplicationTransactionEditorStack doesn't seem to support this easily.
src/applications/dashboard/controller/PhabricatorDashboardEditController.php
20	i really just need panel phids; not sure if worth special casing to save a query.

Yeah, this all looks pretty reasonable to me. I don't think we need to worry about the transactions too much for moving things around -- if it ends up saying something like btrahan moved some stuff around on this dashboard. for all classes of layout changes that feels more than sufficient to me.

I think the other high-level stuff here (like storing this as a big JSON blob on the dashboard, and not trying to mess with edges) is solid too.

Those screenshots practically look like a real dashboard!

src/applications/dashboard/controller/PhabricatorDashboardAddPanelController.php
71–76	I think this is fine as written.
src/applications/dashboard/controller/PhabricatorDashboardEditController.php
20	This seems fine to me as-is.
src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php
28–29	It would be vaguely nice to generalize this so it loops over columns, then over panels, instead of having anything called `$left` or `$right`. But maybe that was on the agenda anyway, and I imagine we aren't really ever going to need more than, like, 4 total columns.
src/applications/dashboard/storage/PhabricatorDashboard.php
39	For consistency, consider `, $default = null`.
48–51	(Provided by Lisk automatically?)

btrahan retitled this revision from Dashboards WIP - add layout mode to dashboards to Dashboards - add layout mode to dashboards.May 16 2014, 1:49 AM

btrahan updated this object.

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

btrahan edited edge metadata.

no longer a work in progress (though probably will need changes from feedback =D)

this works. drag and drop, etc. screenshots in a moment...

The new select stuff

Screen_Shot_2014-05-15_at_6.46.25_PM.png (1×1 px, 371 KB)

Picking an interesting one

Screen_Shot_2014-05-15_at_6.46.32_PM.png (228×494 px, 42 KB)

Just after hitting create I am in "arrange" mode

Screen_Shot_2014-05-15_at_6.46.38_PM.png (1×1 px, 360 KB)

I clicked the big ole box on the right to invoke this one

Screen_Shot_2014-05-15_at_6.46.45_PM.png (1×1 px, 383 KB)

Just after hitting submit. Got here via redirect after the post.

Screen_Shot_2014-05-15_at_6.46.55_PM.png (1×1 px, 436 KB)

Added one on the left

Screen_Shot_2014-05-15_at_6.47.03_PM.png (1×1 px, 505 KB)

Dragged from left to right

Screen_Shot_2014-05-15_at_6.47.07_PM.png (1×1 px, 502 KB)

Clicked on the crumbs to get back here

Screen_Shot_2014-05-15_at_6.47.16_PM.png (1×1 px, 500 KB)

Harbormaster completed remote builds in B500: Diff 21719.May 16 2014, 1:55 AM

delete some commented out JS

chad awarded a token.May 16 2014, 2:01 AM

Harbormaster completed remote builds in B501: Diff 21720.May 16 2014, 2:02 AM

This all seems totally reasonable to me.

src/applications/dashboard/controller/PhabricatorDashboardMovePanelController.php
45	One general thought: are there any legitimate use cases for having the same panel on a dashboard more than once? I can maybe imagine, like: some kind of nav thing that you want at the top and bottom? some kind of "random picture of a cat" and you want 5 of them? These don't seem like very compelling use cases, but could require this logic to be slot/position based instead of PHID based if we eventually want to support it. (I don't think we should bother for now though.)