Page MenuHomePhabricator

Navigation menus should work correctly on mobile by default
Closed, ResolvedPublic

Description

We show side navigation menus and object action menus in significantly different ways on mobile and desktop. In particular, they are hidden behind disclosure buttons on mobile.

Currently, the code supporting this is not very clean. In particular:

  • Navigation menus and action menus need to render twice. Ideally, the desktop and mobile views should be different views of the same DOM nodes.
  • Navigation menus and action menus need to be explicitly attached to work on mobile. Ideally, these should just work by default, and no one should have to remember to hook them up.
  • ApplicationSearch navigation menus duplicate a lot of queries because of how they render. We basically have to load all of the search stuff in the controller and then all of the search stuff again to build the menu.
  • Some of the patterns are just very old, from before we had standard/unified menus.

Navigation menus are a little more involved. I think that code just needs to be generally reorganized so it fits into modern rendering in a more natural way.

Related Objects

StatusAssignedTask
OpenNone
Resolvedepriestley
OpenNone
OpenNone
Openepriestley
Resolvedepriestley
Wontfixepriestley
Resolvedbtrahan
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
DuplicateNone
ResolvedNone
Resolvedepriestley
ResolvedNone
Resolvedepriestley
OpenNone
InvalidNone
ResolvedNone
Resolvedepriestley
Resolvedepriestley
Resolvedchad
Resolvedchad
Resolvedchad
Resolvedchad
Resolvedchad
Resolvedchad

Event Timeline

epriestley updated the task description. (Show Details)
epriestley raised the priority of this task from to Normal.
epriestley added a project: Design.
epriestley added subscribers: epriestley, bttelle, chad.

I use narrow windows in my desktop workflow a lot and appreciate the responsive design. I get that this is still primarily a mobile flow but it would be great if you can continue to keep things working well for narrow desktop windows in any rework. Right now on the desktop a narrow UX mostly works but there are some gotchas. Most annoying is the search bar that only appears on click but not when pressing /.

chad claimed this task.Jan 11 2015, 5:56 PM
chad edited projects, added Mobile, PHUI; removed Design.
chad added a comment.Jan 11 2015, 6:02 PM

I'd want to move these out of crumbs and into ApplicationMenu by default, that is any action link added to a page, also gets built into the mobile menu. Further since some pages have multiple sections of action links, we'd need maybe some label or dividers automatically built as well.

chad added a comment.Jan 11 2015, 6:08 PM

Alternatively could have a little menu on PHUIObjectBoxView appear on each section, though I can't imagine how to make that happen automatically.

Specifically, you want things like "Edit Task" in the application menu?

In the current design of Releeph, you can have 100 of them on a page. I don't have any test data and it's a huge pain to generate it, but roughly it looks like this:

+-----------------------------+
| Pull Request: More Pink     |
+-----------------------------+
| Author: epriestley | Accept |
| Revision: D123     | Reject |
|                    | Edit   |
|                    | Hotfix |
|                    +--------+
|                             |
+-----------------------------+
| This makes everything more  |
| pink, it is urgent and I    |
| promise it is very safe.    |
+-----------------------------+

+-----------------------------+
| Pull Request: More Violet   |
+-----------------------------+
| Author: epriestley | Accept |
| Revision: D123     | Reject |
|                    | Edit   |
|                    | Hotfix |
|                    +--------+
|                             |
+-----------------------------+
| This makes everything more  |
| violet, it is urgent and I  |
| promise it is very safe.    |
+-----------------------------+

+-----------------------------+
| Pull Request: More Indigo   |
+-----------------------------+
| Author: epriestley | Accept |
| Revision: D123     | Reject |
|                    | Edit   |
|                    | Hotfix |
|                    +--------+
|                             |
+-----------------------------+
| This makes everything more  |
| indigo, it is urgent and I  |
| promise it is very safe.    |
+-----------------------------+

<... 97 more of these ...>
`

Moving those actions into the application menu, even with dividers, doesn't seem great, since you'd see:

Accept
Reject
Edit
Hotfix
-----
Accept
Reject
Edit
Hotfix
-----
<... etc, 98 more times ...>

The use case for this interface (essentially, an actionable list view) is that release engineers at Facebook go through this whole queue before every push.

We don't need to keep the UI like that, but if we don't then we need to do something else with it before we can automatically move actions into the application menu.

Another less extreme case is the Diffusion edit interface:

In this case, the options are uniquely named and just shoving them into the application menu wouldn't be unusable, but we'd lose the relationship between options and editing them (instead of clicking the adjacent button, you have to go read through all the options). There are also still a fairly large number of options.

My proposed fix is to convert this menu to a button on mobile:

               New Button --+
                            |
                            V
+-----------------------------+
| Pull Request: More Pink  [=]|
+-----------------------------+
| Author                      |
|   epriestley                |
| Revision                    |
|   D123                      |
+-----------------------------+
| This makes everything more  |
| pink, it is urgent and I    |
| promise it is very safe.    |
+-----------------------------+

When clicked, it would disclose the options:

                   Click! --+
                            |
                            V
+-----------------------------+
| Pull Request: More Pink  [-]|
+-----------------------------+
| Accept                      |
| Reject                      |
| Edit                        |
| Hotfix                      |
+-----------------------------+
| Author                      |
|   epriestley                |
| Revision                    |
|   D123                      |
+-----------------------------+
| This makes everything more  |
| pink, it is urgent and I    |
| promise it is very safe.    |
+-----------------------------+

I like this approach because:

  • It's technically easiest, and I'm confident I can execute it without duplicating DOM nodes.
  • It handles the existing unusual cases gracefully.
  • Keeping actions closely associated with their contextual objects/panels feels good to me.

That said, I'm not especially committed to it. Do you have ideas on the Diffusion and Releeph UIs if we move away from this plan of attack?

Diffusion would probably be OK without changes.

We could maybe just make Releeph work normally (require clicks into it to make changes) and then give it a fast-review workflow in Nuance eventually, although that feels reallllly iffy to me.

Alternatively could have a little menu on PHUIObjectBoxView appear on each section, though I can't imagine how to make that happen automatically.

I'm happy to implement this if you're OK with trying it (this is basically my second "mock", right?), I think it's way easier to execute cleanly from a technical perspective than the other version. If it's garbage we can drop it, but it feels worth trying to me?

chad added a comment.Jan 11 2015, 6:23 PM

Yeah I think #2 is better.

chad added a comment.Jan 11 2015, 7:15 PM

Is this just a hidden button on ActionListView that gets some display: block and negative margin when on mobile? I could probably build that.

Yeah, I think it's just:

  • Button, which is hidden on .device-desktop.
  • Menu is hidden on .device (or just .device-mobile, we might have enough space on tablets).
  • When clicked, button toggles a class on the menu which shows it on .device.
chad added a comment.Jan 12 2015, 2:10 AM

Still not sure how to do this, initial plan was to just push the button up into the header, but I forgot that headers already have their own buttons (not often, but somethings like on the current project homepage). It's still likely the best place for it, just not sure how to make this auto-magical.

chad added a comment.Jan 12 2015, 2:25 AM

Oh silly, I see we're passing PropertyList into ObjectBoxView anyways. Alright, should be able to figure this out after kiddo bath time.

chad renamed this task from Navigation menus and action items should work correctly on mobile by default to Navigation menus should work correctly on mobile by default.Jan 17 2015, 11:04 PM
chad updated the task description. (Show Details)
epriestley moved this task from Backlog to v1 on the ApplicationEditor board.

There are broadly two issues here:

  1. The left navigation menu doesn't do something reasonable by default on mobile.
  2. Property lists in ObjectViews don't do something reasonable by default on mobile.

(1) is my primary focus first.

I think (2) is pretty straightforward, but I'll deal with that after (1).

Actually I guess (2) already got pretty much fixed in D11340.

chad added a comment.Nov 2 2015, 6:24 PM

Happy to help with conversions if that's how this gets solved.