Page MenuHomePhabricator

Add MenuEngine to Home
Closed, ResolvedPublic

Description

PanelEngine has a good bit of flexibility and so do Dashboards. Mixing these two is like mixing curry and vegetables. It just works.

This allows Home to be powered by MenuEngine + Dashboards + Global Links.

Event Timeline

chad created this task.Dec 6 2016, 6:51 PM
chad added a comment.Dec 6 2016, 6:51 PM

Though I feel like we should rename PanelEngine to MenuEngine

chad added a comment.Dec 6 2016, 6:54 PM

CatFactsEngine

chad added a subscriber: epriestley.Dec 6 2016, 6:58 PM

oh @epriestley is at the dentist.

One thought about binding them to dashboards is that users won't naturally be able to edit the menu anymore, to personalize it. But maybe we can put some kind of override mechanism on top of it.

chad added a comment.Dec 6 2016, 7:24 PM

I think fixing the entire dashboards experience should help though. I don't plan to just shoehorn this in.

chad added a comment.Dec 6 2016, 10:18 PM

Do you mind if I rename "ProfilePanelEngine" to "MenuEngine" or something else less vague?

chad added a comment.Dec 6 2016, 10:23 PM

Actually this seems like a lot of work

chad added a comment.Dec 6 2016, 10:24 PM

May be easier to rename Dashboard Panels to Panes or Windows or MacOS

I can rename them, maybe MenuItem instead of Panel? I think when I wrote the stuff we had some more panel-like items in mind (like M1450) but haven't ended up building many of them, at least so far (cat facts and progress bar are the only non-item-like-ones, I think). Even if we do build richer stuff later I think MenuItem is reasonably clear.

chad added a subscriber: cspeckmim.Dec 7 2016, 5:55 AM

@cspeckmim it's gonna get crazy in here.

Also I tried to give you a thorny plant as an offering/warning/omen but it turned out to be a drink. Maybe both were in order.

epriestley renamed this task from Add PanelEngine to Dashboards to Add MenuEngine to Dashboards.Dec 11 2016, 9:29 PM
chad renamed this task from Add MenuEngine to Dashboards to Add MenuEngine to Home.Dec 12 2016, 10:43 PM
chad updated the task description. (Show Details)
kfa added a subscriber: kfa.Dec 13 2016, 6:00 AM

At some point after we get this working for Home, I want to replace the "Quick Create" menu with a similar customizable profile menu, resolving T5867.

@epriestley my assumption here is:

  • Migrate "Global" app layout to Global Menu Item
  • Give up on personalized app layout? Or leave a message?

I think it would potentially be reasonable to not migrate, especially if you do another blog post like the Favorites one to ease migration pain. Global stuff is relatively recent and pretty easy to put back in place, and I think a gentle nudge to configure the menu to work well for your install isn't a bad thing.

I'd just give up on personalized layout. If we migrate it we'll probably get a bunch of junk (duplicate application items that appear in both global and personal menus) and, likewise, easy to regenerate and not necessarily bad to nudge users into exploring the new stuff.

(If there's pushback, maybe like a "one-time hacky copy your old global app settings into the menu script that we don't support" sort of thing? But I'd guess users won't feel like this is too painful since the new menu is a lot more flexible/powerful.)

While I've got you, how do you feel about a diff to:

  • Let "Application" menu items have a custom name (for example, you can label Differential as "Code Review"), like other items can (projects, dashboards, forms).
  • For all of these items, put the required field (Application, Project, Dashboard, Form) first, then the optional "name" field second.

I think the first one is just an oversight/inconsistency, but maybe you had more plans there? And I keep getting tripped up on the second one (I click "Application", start typing "diff" to select Differential or whatever, realize I'm not doing what I want) but maybe I'm alone.

chad added a comment.Jan 20 2017, 7:38 PM

Yeah I meant to clean that up too

chad added a comment.Jan 26 2017, 6:37 PM

I think I have most of this working, but there will be some follow ups needed I'll need help with:

  • Selecting first dashboard as homepage. I think I can just query the menu itself in MainController and pull it out? Use built in home if nothing returned?
  • Ability to ajaxify dashboards.

Selecting first dashboard as homepage. I think I can just query the menu itself in MainController and pull it out? Use built in home if nothing returned?

I think you actually don't have to do anything for this, except make only dashboard items defaultable and handle the case where no defaultable items exist (which might be complex/weird?). You could dodge that case by hard-coding the default homepage at the bottom above "Edit Menu", maybe. Not sure we want to prevent you from removing it, though.

Ability to ajaxify dashboards.

Do we actually need this? Dashboards already load nothing since all the individual panels are ajaxed -- and even less with Quicksand -- and there's no state to preserve? What's the goal?

That is, the rule the menu already uses is already "show the item the user has explicitly selected as the default item, or the first item which they could select as a default, if they haven't selected one yet". This is what lets you pick projects to default to the workboard or project profile, but causes them to default to the project profile.

Links, dividers, applications, etc., can't be selected as a default, so if dashboards are the only type that can be a default, this will work as-is without changes.

One slightly weird interaction here is that you can select a default among both your personal items and among the global items. However, I think these rules actually interact fairly sensibly today for the most part and might not need any additional work.

chad added a comment.Jan 26 2017, 6:54 PM

Do we actually need this? Dashboards already load nothing since all the individual panels are ajaxed -- and even less with Quicksand -- and there's no state to preserve? What's the goal?

I mean if you're on home, and you have 3 dashboards, that you don't leave home. I think we talked about this before and you said that was simple.

I mean if you're on home, and you have 3 dashboards, that you don't leave home. I think we talked about this before and you said that was simple.

When you say "don't leave home" do you just mean that the left navigation menu stays on screen? This is simple (it's the default behavior -- I think you had to turn it off to implement Favorites without the menu!), but does not require AJAX.

If you mean "also, the URL bar does not change from / to /home/view/32/" or "also, the navigation is 4ms faster", we need a thousand hours of JS.

chad added a comment.Jan 26 2017, 7:02 PM

On here I've added a personal dashboard called "home", if I click it I go to dashboard/view/1/, and I don't stay on home. Maybe I implemented the dashboard menu item wrong?

If you just want to diff that when it's ready I can implement the fancier version to get the dashboard rendering on the same page. Looking at the code, it's not quite as simple as I remembered -- I thought we had some similar panels already, but all the stuff we have is more hard-coded than I thought.

Instead of linking to /dashboard/view/1/, the item will link to /home/view/283/, which will render the menu, select item 283 (your dashboard link), and then render content for that item (in this case, a dashboard). Should work equally well on Projects (and user profiles, if we do that). This is similar to how all the management/config stuff automatically shows up "in" those applications already without taking you to a separate "Menu Management" application.

chad added a comment.Jan 26 2017, 7:07 PM

Yeah, this is just polish to me, though it makes dashboards I think way more useful (and likely what I'll want to fix the UX of next).

Yeah, fully agreed that we should stay in Home in terms of the nav menu, crumbs (none on Home, but relevant for Projects), checking application permissions, etc. I just got scared off by "AJAX", since I think the non-ajax thing will take me 20 minutes and an Ajax version would take me 20000 minutes.

chad added a comment.Jan 26 2017, 7:09 PM

I hadn't considered building dashboard routes into /home/ but that seems pretty simple.

chad added a comment.Jan 26 2017, 7:16 PM

proooobably projects too

We shouldn't need to build a route into the applications separately -- getProfileMenuRouting() already adds a route (from the code, I guess the actual route is /home/item/view/123/).

Currently, no menu items actually do anything when you hit this route: they all 404 and/or fatal when they make it to buildItemViewContent() in PhabricatorProfileMenuEngine because I never actually wrote that method. But if that method existed and did stuff and MenuItem had some methods like okayBuildTheViewForThisItem(), it would theoretically all work.

That is, the patch should in theory be something like this, for general support:

  • Link to /<application>/item/view/<item identifier>/.
  • Actually implement buildItemViewContent().
  • Have that call some new method on the MenuItem.

Then for dashboard support:

  • Implement okayBuildTheViewNow() on DashboardMenuItem, and it magically becomes available in every app without changes to the apps.

Then for "picture of a cat" support:

  • Implement okayBuildTheViewNow() on PictureOfACatMenuItem, and it magically becomes available in every app without changes to the apps.

etc. At least in theory.

chad added a comment.Jan 26 2017, 7:32 PM

Oh, I guess you could pre-patch me with projects then

Yeah, but since you already wrote DashboardMenuItem we'd conflict -- do you want to just diff that separately now, then I'll fancy-it-up and it should integrate cleanly?

Or I can give you PictureOfACatMenuItem but this project officially hates fun etc etc

chad added a comment.Jan 26 2017, 7:40 PM

I don't think we'd conflict? Nothing I'm doing is in MenuItem (all that stuff is wired). I'm just deleting code.

Oh! I thought that mock was a screenshot and that "Home" was a new DashboardMenuItem. No conflicts, I'll shoot you a diff in a bit.

I have discovered that we already have PhabricatorDashboardProfileMenuItem.

A quick word to express my satisfaction about the new personal menu: after some days, it becomes standard practice on two instances I use to access current sprint workboard from the menu.

As the previous global menu was heavily cluttered with each form listed, it's really nice to have something we can adjust to pick essentials one.

chad closed this task as Resolved.Feb 1 2017, 12:12 AM

Resolved at HEAD. Follow T12174 for bugs/feature requests/future action items.