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

doc: add a little warning for <param ... multiple="true"> #930

Merged

Conversation

bernt-matthias
Copy link
Collaborator

I stumbled over this multiple times...

Wondering what happens if the user selects multiple collections in such an input field

nsoranzo and others added 2 commits May 5, 2019 17:03
nsoranzo and others added 3 commits May 5, 2019 17:16
Co-Authored-By: bernt-matthias <m.bernt@ufz.de>
Co-Authored-By: bernt-matthias <m.bernt@ufz.de>
- to access properties of the collection (name,...), or
- to write tests that pass a collection to the parameter (which would allow to name the elements explicitly).

Another drawback is that the `${on_string}` of the label contains the list of data sets in the collection (which can be confusing, since these data sets are in most cases hidden) and not the name of the collection.
Copy link
Member

@nsoranzo nsoranzo May 6, 2019

Choose a reason for hiding this comment

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

That's probably something which we may want to fix?

galaxyproject/galaxy#7467

@@ -125,13 +125,22 @@ Processing Lists (Reductions)
-------------------------------

The ``data_collection`` parameter type can specify a ``collection_type`` or
``list`` but whenever possible, it is actually better to not explicitly
``list`` but whenever possible, it might actually be better to not explicitly
Copy link
Member

Choose a reason for hiding this comment

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

This represented an IUC recommendation... I think I agree with the language changes here - but I was always more pro-collection inputs then everyone else. So I'm nervous about making this less of a recommendation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. I changed the wording to "it is recommended".

Co-Authored-By: Nicola Soranzo <nicola.soranzo@gmail.com>
@jmchilton jmchilton merged commit 728660a into galaxyproject:master May 15, 2019
@jmchilton
Copy link
Member

Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants