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

[WIP] Workflow Run Form Enhancements #19294

Draft
wants to merge 21 commits into
base: dev
Choose a base branch
from

Conversation

ahmedhamidawan
Copy link
Member

@ahmedhamidawan ahmedhamidawan commented Dec 9, 2024

Fixes #18883

Description from commit f77b387

  • This drastically changes the styling of FormElements as they have badges (or full colored headers) along with title sections, indicating current status of a step. Each step is in a card like style.
  • The variant for what type of FormData option it currently is has text now, letting the user clearly know whether they are selecting a single dataset, multiple etc.
  • After a "browse section" in the FormData component, we have a tabs section which shows the users the current selected file(s), as well as providing options for uploading/creating collections etc in other tabs.

Note: Towards the end of this I show an alternate implementation for each step header, where we have a badge instead of the whole header being colored:

simplified_wf_run_2.mp4

Initial version screencast which showcases the graph view for the run form:

simplified_wf_run.mp4

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

mvdbeek commented Dec 10, 2024

I don't think the split view is a good idea. This will never work for production-sized workflows and makes the whole submission harder to understand. If there is a side-panel control option (one that hides the activity bar and history) I would split that out as its own enhancement, and one that we'd probably want to be able to set via the landing request API as well.

@ahmedhamidawan
Copy link
Member Author

Some updates made here, a screencast has been added to the main description

@ahmedhamidawan
Copy link
Member Author

There is also this version where it is a top down + graph view, where you fill each input one by one:

simplified_wf_run_top_down.mp4

@mvdbeek
Copy link
Member

mvdbeek commented Jan 21, 2025

The wizard like fill in menu is great, the graph however is not useful and should only be shown to advanced users on the click of a button or a toggle. For a good workflow the graph should never be needed, and if you're debugging an existing workflow I think you want the entire available screen estate.

@ahmedhamidawan
Copy link
Member Author

ahmedhamidawan commented Jan 21, 2025

Thanks for taking a look! Yes, the graph is only viewable with a toggle if needed. And I see the point about not needing the graph for context for a very good workflow.

A minor point:

Do we like this badge styling for current step status?: (👍 to +1 this)
image
Or these fully colored headers?: (🚀 to +1 this)
image

@bgruening
Copy link
Member

Humans are often not very pragmatic but often like to see nice, coloful things and get an overview that looks awesome. If we link to this run-page from BRC and similar sites, I think it is a very nice feature to show the workflow by default - even if its just bling bling. We can still hide it with a button and safe this user-choice imho.

@mvdbeek
Copy link
Member

mvdbeek commented Jan 21, 2025

this run-page from BRC and similar sites, I think it is a very nice feature to show the workflow by default -

I don't mind making this an option, but BRC is a good example where this should definitely not be shown.

@ahmedhamidawan
Copy link
Member Author

Right now, ONLY IF hide_panels=true is in the route query, we do initialize the run form with showGraph = true by default, since we know we have the space for both the inputs and the graph, but for ALL other cases, the graph is hidden and there is a toggle at the top which allows you to show/hide it and IF shown, allows for an additional toggle for top-down vs side by side view.

@mvdbeek
Copy link
Member

mvdbeek commented Jan 21, 2025

That's an unfortunate coupling, we don't need panel or graph in BRC. I would much prefer if the graph is never shown by default, this just makes running the workflow much more complex than it needs to be.

@ahmedhamidawan ahmedhamidawan linked an issue Jan 23, 2025 that may be closed by this pull request
@ahmedhamidawan
Copy link
Member Author

README in Run Form:

In the latest commit I've added a README section only for dockstore workflows from IWC like so:

simplified_wf_run_READMEs.mp4

just syncs the active node id with the graph for now
The input values are synced with the graph
- We show the appropriate, typed step description for unpopulated or errored steps on the `WorkflowRunGraph`
- Changed the `stepValidations` object to an array of `[index, msg]` since we can use the return of `validateInputs` as is, and hence, track only the latest received error message.
- This drastically changes the styling of `FormElement`s as they have badges (or full colored headers) along with title sections, indicating current status of a step. Each step is in a card like style.
- The variant for what type of `FormData` option it currently is has text now, letting the user clearly know whether they are selecting a single dataset, multiple etc.
- After a "browse section" in the `FormData` component, we have a tabs section which shows the users the current selected file(s), as well as providing options for uploading/creating collections etc in other tabs.
and improve styling and positioning of the side by side graph view
Fixes galaxyproject#18815

... at least, as far as being able to drag and drop to individual workflow steps in the run form is concerned.
@ahmedhamidawan
Copy link
Member Author

From UI/UX meeting:

  • Move accepted formats to header
  • Make step header shorter in height
  • Revert to badge styling for step state
  • By default, hide FormDataWorkflowRunTabs
    • Change it from tabs to just the uploader/creator
    • IMO, move current value outside of it (above it)
    • Put a toggle (on the right of the form selector?) to expand the uploader/creator below
  • Break PR down into smaller iterations...

When you switch from singular to multiple variant, the `currentValue` variable can get the same dataset/collection/value twice
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.

Simplified workflow run interface Enable dropping files into data and collection inputs
3 participants