Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Modernize workflow editor state handling #15268

Merged
merged 179 commits into from Jan 9, 2023

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Jan 3, 2023

There's a whole lot this PR is doing (sorry about that, I'm contemplating pulling out smaller parts), but these are the main points:

  • All state is reactive and stored in pinia stores, that means what you see in the editor is really what will be persisted
    • I want to improve this further by injecting the right, workflow id scoped stores at the top index level, so we can work with multiple open workflows
    • (I added a monaco instance that displays a cleaned-up version of the step definitions to look at the state, I'll drop this later)
  • Steps have a fairly complete typescript interface (excluding tool state)
  • We don't pass around vue instances into child instances anymore
  • No jQuery left in the workflow editor (except for tool form parameters ?)
  • Smooth zooming and panning is handled by a custom d3-zoom wrapper
  • Step, terminal positions and connections are reactive
    • I think that'll be needed for multi-select dragging of nodes
  • I've ported the connectability logic in terminals.js to typescript and eliminated all dom manipulations and accessing of vue instances (test coverage is > 99% for this module)
  • A lot of composition API and typescript conversions

This should be a good start to add new workflow editor features, like selecting and manipulating multiple steps, editing multiple workflows, displaying invalid connections, displaying conditional steps and connections and more.

TODO:

  • reimplement loading span when step is being updated
  • tool recommendations
  • drop pan indicator
  • drop monaco
  • use upstream @vueuse/core
  • reimplement scroll to node
  • reimplement active checkbox -> workflow output logic
  • collection output icon should be enlarged, currently only enlarges collection input icons

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@mvdbeek
Copy link
Member Author

mvdbeek commented Jan 5, 2023

I've opted to split out the workflow output button from the output visibilty, that fixes an open bug I'll dig out later. I know this is a little controversial, let me know if y'all have better ideas.

output and visibilty

I will leave in the pan indicator and monaco for the reviewers, that's easy enough to drop out once I have a 👍

@mvdbeek mvdbeek marked this pull request as ready for review January 5, 2023 14:28
Copy link
Member

@dannon dannon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fantastic step forward. I noticed a few minor things we can follow up on during the freeze, but this is working very well for me and I'd like to go ahead and get it in. Really great work @mvdbeek

@dannon dannon merged commit 328e743 into galaxyproject:dev Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants