On Dashboard tab panels in edit mode, make the "Tab Name" and the "Dropdown Edit Caret" into different links
ClosedPublic
Actions

Authored by epriestley on Apr 11 2019, 3:06 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T13272: Remove the hard-coded homepage, and other Dashboards improvements

Commits

rP9ad9ac9be695: On Dashboard tab panels in edit mode, make the "Tab Name" and the "Dropdown…

Summary

Ref T13272. In edit mode, tab panels now have a dropdown menu. However, this sort of overrlaps with the actual action of clicking the tab to select it.

Separate these into different click targets so that "select tab X" and "open dropdown menu for X" are different operations.

This is more work than it appears because:

We have an "action icon" already, used when you put a dashboard on a portal/home to create an "Edit" link. It makes sense to attach dropdowns to this, but it has some hard-coded stuff.
In applications with a "Create <thing>" in the crumbs (like Maniphest), we may use a dropdown menu if there are multiple create forms available. However, this menu renders in a weird way by reading all the properties out of an actual "View" object and building something else.
The "list of tabs" stuff shares code with different "list of tabs" navigation used by Diffusion and Instances.

..but I think I fixed everything and didn't break anything.

Test Plan

Clicked "select tab" and "open dropdown menu" as separate actions.
Viewed Diffusion, Maniphest with multiple create forms, Instances.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.Apr 11 2019, 3:06 PM

Harbormaster completed remote builds in B22547: Diff 48674.Apr 11 2019, 3:07 PM

epriestley requested review of this revision.Apr 11 2019, 3:07 PM

epriestley added a child revision: D20397: When editing a tab panel from a dashboard, redirect back to the dashboard.Apr 11 2019, 4:27 PM

amckinley accepted this revision.Apr 11 2019, 6:16 PM

amckinley added inline comments.

src/view/phui/PHUICrumbsView.php
61	I'm sure there's an obvious reason why we can't make a lint rule that detects "incorrect" indentation, but it's escaping me. I know we already enforce requiring braces instead of single-line blocks... Is it just that we don't want to mandate the number of spaces in an indent, which means we'd have to detect the "intended" number before enforcing things?
src/view/phui/PHUIListItemView.php
254–255	haha I don't think I've ever seen a `//TODO` in an exception before

This revision is now accepted and ready to land.Apr 11 2019, 6:16 PM

epriestley added inline comments.Apr 11 2019, 6:21 PM

src/view/phui/PHUICrumbsView.php
61	No technical reason, it's just a lot of work to write the rule because there are so many cases. Not weird special cases or anything, just a lot of actual language constructs that affect indentation so you have to enumerate all the different control statements and `class` and `switch` and `try/catch` and so on.
src/view/phui/PHUIListItemView.php
254–255	Yeah, this is a little sketchy but I didn't want to leave us with a silent failure when you configure the View in an apparently-correct way that magically doesn't work.