Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Proposal for new editor navigation model #1823

Open
glopesdev opened this issue May 26, 2024 · 1 comment · May be fixed by #1870
Open

Proposal for new editor navigation model #1823

glopesdev opened this issue May 26, 2024 · 1 comment · May be fixed by #1870
Labels
proposal Request for a new feature
Milestone

Comments

@glopesdev
Copy link
Member

glopesdev commented May 26, 2024

The current editor navigation model has proven unwieldy for large, deeply nested, workflows.

This proposal is for a simplification of the navigation model which should be more scalable, and hopefully require minimal modifications to the current implementation.

Detailed Design

Basic Principles

  • Only one workflow graph view at a time is visible and active in the canvas;
  • Navigating to a nested workflow switches the view to render the new workflow;
  • Breadcrumbs or tree view control assists navigation across the hierarchy.

Essentially the analogy here is to a browser, where navigating hyperlinks to other pages simply opens and renders the new page in the same window by default.

There are many implications, opportunities for redesign, and performance optimisations, that can result from this proposal. Below we try to collect and describe them in more detail as we analyse the proposal.

Render Logic

The GraphNode class is the backbone for representation of workflows in the editor controls. GraphNode objects are currently stored and manipulated in many places, including the undo/redo stack and visualizer associations. The complexity of keeping track of this joint state and the associations between graph nodes and expression tree nodes have been a frequent source of bugs and tangled logic.

This proposal is an opportunity to remove this representation strategy entirely, and adopt a purely functional approach where graph nodes are created and used simply for the current render layout state, and discarded as soon as the structure changes.

Visualizers

Visualizers were previously associated with editor view graph nodes, but in this model it might be simpler to associate them directly with the expression builder nodes, using a global map.

This will make it easier to have all visualizers be children of the main window, which will resolve the issue of nested workflow editor windows having to be open to display deep visualizers.

Error Highlighting

Both compiler and runtime errors were previously reported by opening nested editor windows in order to reveal the innermost cause of unhandled exceptions. This was clunky as it led to a potential cascade of new editor windows.

Under this proposal, we can reimagine error highlighting by leveraging the tree view, and simply place a mark next to the hierarchy levels which are reporting errors. Opening the corresponding level will highlight any error nodes contained within.

Outstanding questions

Include Workflows

How should IncludeWorkflow operators be displayed in the tree view? Currently we were aiming to strike a balance between using them as building blocks (no need to navigate into them) and organization (components of a larger system which may be navigated into quite often).

Disabled Workflows

How should disabled group workflows be handled? A few options:

  • Handle disabled groups as a primitive non-group node, so you cannot explore them
  • Render the internals as either all nodes disabled, or with some kind of annotation marking the whole workflow as disabled
  • Limit expansion the same way as include workflows (require explicit Go To Definition) and make them read-only

Undo/Redo Stack

Currently navigating into or out of a nested workflow was being logged as an undo/redo command. Under this proposal it might make more sense to have only changes to the workflow be logged in the stack, however it is still required to show the undo/redo changes in the context of the workflow level which is changing. This should be straightforward assuming there is always a way to immediately navigate to the workflow which is currently being modified.

Navigation paths

Related to both the above and the serialization of visualizer layout files and visualizer mashups, there is a possibility of specifying "navigation paths" which would allow selecting or opening any specific node of the workflow at any nesting level. Such paths might be serialized in the .layout file, allowing selective serialization of only those visualizers which are actually open or assigned, rather than the current strategy where all visualizers of active layers are stored, even if they have never been opened.

It would have been nice here to include the option for "querying" the node by name. Unfortunately this is hard since we have currently no constraints whatsoever on node names, and it would make the parser too complex.

One option to work with non-sanitized paths existing right now would be to treat any group node names with invalid characters as if they had no name, e.g. Task Controller (SelectMany) would be considered to have the name SelectMany.

Only group node names should be included in navigation paths, i.e. node names coming from INamedElement are not allowed. The only exception might be IncludeWorkflow operators where we might want to use the name of the included operator, assuming it is sanitized.

Draft proposals for "ideal" navigation path:

Purely indexed paths

0.3.23

Hybrid name + index

#Hardware.SelectMany[3].Behavior[2]

Indices only in leaf nodes

#Hardware.TaskController.Behavior[2]
@glopesdev glopesdev added this to the 2.9 milestone May 28, 2024
@glopesdev glopesdev added the proposal Request for a new feature label May 28, 2024
@bruno-f-cruz
Copy link
Contributor

bruno-f-cruz commented May 28, 2024

Breadcrumbs or tree view control assists navigation across the hierarchy.

It would be interesting to try to leverage this feature to allow "path-like" references to places in the workflow.

Essentially the analogy here is to a browser, where navigating hyperlinks to other pages simply opens and renders the new page in the same window by default.

I agree with the default! Notwithstanding, the option to open in a new, detached, window should be preserved.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
proposal Request for a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants