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

Workflow run display improvements - esp. for inputs. #3369

Merged
merged 9 commits into from Jan 8, 2017

Conversation

Projects
None yet
5 participants
@jmchilton
Copy link
Member

commented Dec 26, 2016

  • If steps have labels, use them.
  • Use workflow step type icon from workflow editor in the run step bar.
  • Eliminate "Step Index" from display of workflow steps.

Given the following labelled workflow input:

workflow_icon_0

It would show up as follows in the run form:

workflow_icon_2

I think the fact the label is ignored is problematic. This PR fixes that. It also condenses the display of the input a bit - using the icon used in the workflow editor to describe the "type" of step represented instead of a potentially redundant textual description of the workflow step.

I also don't really like the "Step Index" display in the title taking up so much real estate. It isn't something users really need to know about and it is confusing in my opinion to describe an input as a "step" of the workflow so I just removed that outright. The new more condense, informative input is as follows:

workflow_icon_3

jmchilton added some commits Dec 26, 2016

Rework display of inputs for workflow form.
- If steps have labels, use them.
- Use workflow icon from workflow editor in step bar.
- Eliminate "Step Index" from display of workflow steps.
@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Dec 27, 2016

@galaxybot test this

@erasche

This comment has been minimized.

Copy link
Member

commented Dec 27, 2016

Fwiw, my org's documentation for "how to run this workflow" includes a couple of steps with variable parameters and users are instructed to change parameters in those steps.

utvalg_007

While in all of my documentation instances, the steps are relatively uniquely named (and the forms expanded by default during run), I could see the removal of step number being a negative. (E.g. we have three blast steps in a row with only small changes between them. If there were some parameter our documentation told them that they had to change, we couldn't easily instruct the user exactly which step was which.)

Might be nice to hear from others but some alternative solutions:

  • Remove "Step ", leave the number
  • de-emphasize step (remove boldness)
  • Move step number to the right side if it is visually disruptive.

Otherwise, SUPER excited to see input dataset label being used. That is so much clearer! Also like the icons. They're subtle but appreciated.

@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Dec 27, 2016

@erasche I like removing the step but keeping the number - that might be a good compromise. Happy to hear from others and just to restore the previous prefix if it is desired also.

@erasche

This comment has been minimized.

Copy link
Member

commented Dec 27, 2016

Personally, I'm happy to see "step step step step" gone, it was definitely redundant!

@guerler

This comment has been minimized.

Copy link
Contributor

commented Dec 27, 2016

I think its a good idea and improves the display, but the changes to form-input.js and form-section.js should not be necessary, right?

@guerler guerler force-pushed the jmchilton:workflow_inputs_display branch from 71113b3 to cd0387f Dec 27, 2016

@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Dec 27, 2016

@guerler Are you saying there is a different way to do it or that there is no change needed? It would just show like null or false in text for me without these I thought. I was just sort of trying different things to get it to work though - maybe not all these changes are needed. Feel free to push that change directly or I can play around with it some more.

Merge pull request #51 from guerler/workflow_inputs_display_000
Undo changes label handling adjustments to form-input and form-section modules
@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Dec 27, 2016

@guerler Opps - sorry, missed the PR. Yeah it seems to work - I must be crazy.

guerler added some commits Dec 27, 2016

@nsoranzo

This comment has been minimized.

Copy link
Member

commented Dec 28, 2016

Agreed with @erasche that step numbers are useful.

I like removing the step but keeping the number - that might be a good compromise.

+1

Refine workflow step title display in run form.
- Go back to prefixing with the step number as discussed on #3369.
- Use the label regardless of step type - don't ignore tool labels.
@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Dec 28, 2016

Based on this conversation, I have updated this to add the prefix of the order index back on the step title display.

@bgruening

This comment has been minimized.

Copy link
Member

commented Jan 8, 2017

@galaxybot test this

1 similar comment
@bgruening

This comment has been minimized.

Copy link
Member

commented Jan 8, 2017

@galaxybot test this

@bgruening bgruening merged commit bcd660c into galaxyproject:dev Jan 8, 2017

4 checks passed

api test Build finished. 244 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 132 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 580 tests run, 0 skipped, 0 failed.
Details
@bgruening

This comment has been minimized.

Copy link
Member

commented Jan 8, 2017

Merged manually because of merged in JS bundles.

@bgruening

This comment has been minimized.

Copy link
Member

commented Jan 8, 2017

Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.