nasa / openmct

A web based mission control framework.
https://nasa.github.io/openmct/
Other
11.98k stars 1.24k forks source link

[Context Menus] Specify what context influences menus #138

Closed VWoeltjen closed 8 years ago

VWoeltjen commented 8 years ago

Describe what actions should appear (and should not appear) in context menus in the various contexts where context menus appear.

Note that most desirable outcome here is not just "what appears where" but more of the "why" - that is, what are the things that describe a context in a manner that influences the presence/absence of actions? This will be useful architecturally to make sure these things are passed in when popping up a context menu, and will also be helpful going forward as new actions are added in the future.

charlesh88 commented 8 years ago

@VWoeltjen @akhenry @larkin Guys, please have a look at pages 45 and 46 of the attached UI v2.1.4 sketches for my first take on more detailed context menu defining for this issue. I consider this an opener in an ongoing conversation, so looking forward to your responses. Note that I have NOT updated Google docs due to the pending changeover.

UI WARP v2.1.4 sketches.pdf

VWoeltjen commented 8 years ago

Thanks for getting this conversation started! I've taken a stab at defining some rules which would result in those context menus; this is a straw man as much as anything. There are many other rules that could offer equivalent results.

Sticking with the convention here that context menu contents are controlled subtractively rather than additively (actions are available unless excluded by some policy); this is consistent with current implementation and friendlier to plugin developers.

New property of actions:

Context menu contents are filtered based on:

Policies:

  1. Actions in groups "mutation" and "composition" are disallowed for objects which disallow modification.
  2. Actions "Edit" and "Edit in place" are disallowed for objects which disallow modification.
  3. Modification is disallowed for telemetry points.
  4. Action "Edit" is disallowed when already in Edit mode.
  5. Action "Edit in place" is disallowed when already in Edit mode and object is already selected for sub-object editing.
  6. Actions in the "navigation" group are disallowed while in Edit mode.

Correlation to diagrams:

larkin commented 8 years ago

Charles, I like the basic jist here: "we should suppress actions where they don't make sense, show them where they do make sense, etc." Points to a more flexible infrastructure for building and displaying menus.

Victor, I'd kick back that "Groups" would be another level of configuration when we don't need it. Controlling the order of actions in any given menu can be achieved by priority (and individual menus should be free to reorder actions if necessary), while controlling the actions visible in a menu is handled via policy (and again, a specific menu should be able to arbitrarily include/exclude options). I already feel policy is borderline excessive and given that we haven't even fully documented it's usage; I would rather we polish and refine the existing concept before introducing a new one.

I think the underlying feature I'd like to allow is for context menu to be something you can add to any given element-- a directive that allows you to specify the domain object, as well as a filter on the available actions. Such that a view controller (say, a tree-node controller) can specify actions that are available for a given node. Allows the author of a view/controller to inject one-off actions without having to learn anything about the larger concept of policies (but still allows them to utilize policy should it have value for them).

VWoeltjen commented 8 years ago

Victor, I'd kick back that "Groups" would be another level of configuration when we don't need it.

Groups are in the UI docs, insofar as related actions are visually grouped. So we "need it" in some manner to implement that design.

If groups are semantic then it is also valid to use them in application logic to simplify policy decisions. This avoids introducing redundant concepts (but is contingent upon acceptance that visual grouping and semantic grouping are indeed the same.)

It also saves us from writing redundant policies. My "developer use case" here looks something like:

  1. I write a new action that influences navigation state.
  2. I see my action in the application, but it's not next to all of the other actions which influence navigation.
  3. I read the docs and make whatever change I need to make to get it to show up where I'd like.

If my Step 3 is "assign it to a semantic group", great, I'm done. If my Step 3 is "assign it a priority", a couple of weeks later I get a bug report saying "Action X appears erroneously in context menus while Editing."

Of course, these are not the only two options.

I already feel policy is borderline excessive and given that we haven't even fully documented it's usage; I would rather we polish and refine the existing concept before introducing a new one.

I think I see where you are coming from with this; I don't consider "group" part of the policy model (used by, sure, but not part of), but understand that a plugin developer who sees an action excluded or included based on some "group" assignment would see it that way.

However we model it, there is some logic that considers a certain set of inputs and yields context menus. That logic should reflect both design rationale and user expectations; otherwise, we are correct by coincidence at best.

I'm not convinced that grouping accurately captures the design intent of, for instance, the phrase: "Context actions during editing should focus on actions pertient to editing and suppress actions that would remove the user from Edit mode." Anticipating the side effects of actions is not possible and forcing actions to declare their side effects is impractical (and more complicated), though, so groups are the best approximation I see.

I think the underlying feature I'd like to allow is for context menu to be something you can add to any given element-- a directive that allows you to specify the domain object, as well as a filter on the available actions. Such that a view controller (say, a tree-node controller) can specify actions that are available for a given node. Allows the author of a view/controller to inject one-off actions without having to learn anything about the larger concept of policies (but still allows them to utilize policy should it have value for them).

I think this is a good idea and agree that this would be useful. I'd go a step further and say we should support menus that have nothing to do with domain objects (a directive where you pass an array of actions, for instance.)

Are you suggesting that we use this mechanism as part of how we implement the rules from the UI docs? I'd be open to this, so long as it reasonably reflects the underlying logic of the design (e.g. which given elements will be responsible for what rules?)

charlesh88 commented 8 years ago

On menu item groups, the grouping is mostly intended to organize related actions together. This gives the user extra context to figure out what an unknown menu item might do, and makes it easier to get to and start using the actions they're interested in. Accentuating current contextual actions that are more relevant and suppressing ones that don't make sense is secondary. And although I suppose a plugin developer might want to, groups and the order of items in a group should not change from context menu to context menu - users get use the organization of items in a menu for things that are alike, and would find that confusing.

With the scale of menu items we have currently, I don't think that grouping is a P1 Must Have right now, but one thing we really do need is the ability to specify the order of menu items. Delete, for example, should always be the last item. I think Pete's suggestion to use Priority for this is the right way to go.

charlesh88 commented 8 years ago

355 is a related task for grouping.

larkin commented 8 years ago

@charlesh88 to clarify what I meant by suppress/show; I literally mean not showing a specific action in a specific context. Example: the "Open in new window" action should not show when a user right-clicks on a sub-object in edit mode. This is functionality we currently lack, and it's made more complex by our current action architecture.

I agree: current number of menu items reduces priority of implementing grouping.

@VWoeltjen That's what I'm thinking-- the viewcontroller has final say in what context menus are available. The edit view can override/filter actions for it's menu, or it can choose to use the default implementation. To support this, we could make "context-menu" a directive which has some optional scope parameters -- support an object as an argument which is passed to getActions, a function which is called instead of getActions, etc.

As opposed to an implementation that based on the current UI docs, we should architect the menus as components-- building blocks of an application, and ensure it's easy to reorganize them at will, should the UI docs change, or unknown use cases arise.

The developer use case you cite is a platform-level developer use case: we should support it, but we need an easier pathway for developers who are working on smaller components.

The developer use case I forsee is this:

  1. I need to add an action to (specific location)
  2. Okay, so I just specify a "getAction" call, instantiate my custom action object, and push it onto the array.
  3. ... I've done that and now I see my action! Success.

And then some time down the road... (or maybe never)

  1. I want to use that action elsewhere.
  2. I register my action with the action service, and now I see my action in places I didn't want it.
  3. I write a policy to only show my action in specific places, great.
VWoeltjen commented 8 years ago

With the scale of menu items we have currently, I don't think that grouping is a P1 Must Have right now,

That makes sense. I don't think grouping is a great approximator for the correct set of actions to suppress, so I think it's safe to discard that idea for now.

As opposed to an implementation that based on the current UI docs, we should architect the menus as components-- building blocks of an application, and ensure it's easy to reorganize them at will, should the UI docs change, or unknown use cases arise.

I agree with this, and think the developer use cases you've cited are valid to support.

I do think that still leaves us with issues about how well we can control the validity of actions with policy. Again, I'm thinking of "suppress actions that would remove the user from Edit mode". This rule isn't tied to any specific UI component. Currently, we don't have any way of describing that rule in one place within the code. That means if we enforce the rule, we do it in many places. If that rule changes or evolves, we fix it in many cases. And even if that rule doesn't change or evolve, we still have to remember it when we add new actions (next week, next month, next year.)

I'll take a look at Eclipse RCP and other similarly deep UI toolkits/frameworks and see if there are good solutions out there. In any event, this is getting a little bit off-path for this issue (which is just trying to figure out what the appropriate inputs to those policy decision are.)

akhenry commented 8 years ago

Actions already have a "category" attribute. If we introduce another way of semantically grouping actions, we need to be careful to avoid confusion here. I realize that grouping has been parked for now though.

VWoeltjen commented 8 years ago

Notes on other frameworks which handle this:

Framework How are actions introduced? How are they filtered? How is "context" established?
Eclipse RCP * At named extension points Via a visibleWhen clause Via a contextService singleton (data inside is application-specific)
NetBeans At named extension points Unclear; appears to be by reflection (can constructor be satisfied by context?) in the general case Graph of "nodes"; selections distinguished by class
Mozilla Add-ons Imperatively (action descriptions are somewhat declarative) By declared context dependencies Appears to be a bounded set of typed dependencies (URLContext, PageContext, etc)

* Note that Eclipse RCP also provides imperative approaches for these.

In general, UI toolkits (qooxdoo, Swing, etc) don't try to solve this, and the contents of a context menu are left for the application to manage.

Think our case is most like Mozilla Add-ons ("estensible application" as opposed to "rich client platform"); also like that this is less dependent on string-matching (no "named extension points") which we have been moving away from.

This leaves filtering and acquisition of context data. I like the dependency-driven approach, and the idea of having some model for context that is both generalizable and extensible. Should do this in a way that allows us to refine the specific things that qualify as "context" in the future. Currently, "domain object" is really the only thing here

Additionally, want to provide loose coupling between context menus and the infrastructure which drives their contexts. Should be as simple as new ContextMenuComponent(actions), where actions is an array of Action objects (perform, getMetadata.) This makes it simple for special cases to have one-off sets of actions that appear in these menus. Whatever infrastructure handles the general notion of context and filtering should be upstream of this, and its usage should be optional.

Want to handle the specifics of that infrastructure in the context of #463 (or later); more immediate goal should be decoupling context menus from whatever context/filtering infrastructure exists.

VWoeltjen commented 8 years ago

Think we have the outcomes we want for this issue (don't have a specific notion of what falls under "context", but have described a set of approaches which will allow us to get desired results without this notion, as well as a path for developing this concept as we go.) Marking resolved.