Page MenuHomePhabricator

Rebuild Dashboards on EditEngine: v1 Major Jank Edition
ClosedPublic

Authored by epriestley on Tue, Apr 9, 8:02 PM.

Details

Summary

Depends on D20383. Ref T13272. Fixes T12363. See PHI997. This gets the edit flows for tab panels functional again. They aren't nice, and a lot of the workflows are fairly janky: for example, most of them end up with you on the tab panel's page, which isn't useful if you started on a dashboard page.

However, these flows were extremely janky before anyway (see T12363) and I suspect this is a net improvement even though it's a bit of a mess. I anticipate cleaning this up bit-by-bit in future diffs.

Test Plan

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Tue, Apr 9, 8:02 PM
epriestley requested review of this revision.Tue, Apr 9, 8:04 PM

There are a couple of bugs with the profile menu stuff in master that I've fixed locally (notably: https://discourse.phabricator-community.org/t/call-to-undefined-method-getprofilemenu/2614/) but I've been waiting to land everything else until tab/query panels aren't totally broken. So my major goal here is a minimally functional tab panel that lets everything in queue land to fix the profile menu stuff, and then followups can clean up some of the behavior (like adding "move tab left / right", redirecting to the right place after an edit, allowing tab editing from panel detail view pages, making the Javascript feel around swapping tabs vs opening dropdown menus less sketchy, making "Create new panel" work within a tab panel, and so on).

As-is, this feels pretty half-baked, but this is what we had before (not pictured: if you skip any fields the entire form breaks) so it isn't exactly replacing some rare masterpiece:

Broadly, I'm trying to move toward a more visual/contextual editing flow where you use dropdown menus to "Add Tab", "Rename Tab", "< Move Tab That Way", and so on.

epriestley edited the summary of this revision. (Show Details)Tue, Apr 9, 8:29 PM
epriestley added inline comments.
src/applications/dashboard/typeahead/PhabricatorDashboardPanelDatasource.php
57

This may break "Add Existing Panel" on Dashboards, but the change (setPHID(phid) instead of setPHID(id)) is obviously correct and that's the next major thing I'm planning to fix.

amckinley accepted this revision.Tue, Apr 9, 10:03 PM
amckinley added inline comments.
src/applications/dashboard/controller/panel/PhabricatorDashboardPanelTabsController.php
96

Not just throw new UnimplementedException()?

This revision is now accepted and ready to land.Tue, Apr 9, 10:03 PM
epriestley added inline comments.Tue, Apr 9, 10:35 PM
src/applications/dashboard/controller/panel/PhabricatorDashboardPanelTabsController.php
96

You can't actually hit this right now without editing the URI, but it turns into a "controller didn't do anything with this request" exception with mostly the same flavor as NotImplemented.

This revision was automatically updated to reflect the committed changes.