-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[24.2] Guide users to collection builders #18857
[24.2] Guide users to collection builders #18857
Conversation
For anything else. I would make that a follow up, it isn't that common of a starting point |
Valid implicit conversions are defined by converter targets in the datatypes_conf.xml file (see https://github.com/galaxyproject/galaxy/blob/dev/lib/galaxy/config/sample/datatypes_conf.xml.sample#L106 for the csv -> tabular conversion), not the class hierarchy. Direct matches without conversion are defined by the class hierarchy. Both should be handled correctly in current code, so if you need to write new logic there is something odd going on. |
I don't think it's wise to let users choose between these. We don't do that elsewhere in the interface, and users shouldn't care.
That should only be possible if all extensions are valid for an input. This is a recipe for extremely hard to debug problems. I've spent months of my life narrowing this down and greatly reducing tool templating errors. |
I guess this is exactly what I was looking for; something that can indicate which items could be valid implicitly converted inputs for the list given the required explicit extensions. The Based on my reply to your review earlier, would it still make sense to add this logic to Update:I have removed this optional choice between the level of filtering and enforced only allowed extensions (and others that are |
7d474c2
to
12f6957
Compare
client/src/components/Form/Elements/FormSelectMany/worker/selectManyMain.ts
Outdated
Show resolved
Hide resolved
53e5995
to
a69b79a
Compare
They've passed now, the other tests like Mulled Unit Tests, Test Galaxy packages for Pulsar, Toolshed tests etc seem unrelated to these changes |
cb18589
to
5090d7b
Compare
This is an initial/draft implementation. Some of the next steps are: - By default, instead of the modals treating the items as selections from the current history, automatically filter items valid for the list (e.g.: for a list with csv elements, filter out csvs from the history in this list). - In case nothing can be auto paried for `list:paired`, do not attempt to auto pair by default and simply show all items. - In case the current history is empty and to make it clearer in general, allow history to be switched from within the modal? - Allow files to be uploaded (and dropped) directly to either the form field or within the list builder once it is opened. One thing I have not planned for yet is the rule builder. I can see that for `list` and `list:paired`, we get that from the `props.collectionTypes` in `FormData`. But when would we use the rule builder instead? Fixes galaxyproject#18704
This allows a collection type `list` to be created via the collection creater from the workflow/tool form directly. It tracks the current history changes via the new `useHistoryItemsForType` composable. It utilises the `FormSelectMany` component to easily move items between selected and unselected for list columns. The items in the list creator can be filtered for extension, parent datatype or all items in the history, based on whether the form field required a certain extension(s) as input for the list.
This keeps the order in which the user adds items to the selection evident in the selected column.
- Converted the file(s) to composition API and typescript - Improved styling of the modal and its components Still need to add `extensions` handling for cases where a certain extension is required for the collection.
The `isSubClassOfAny` was incorrect logic for implicit conversions. `isSubTypeOfAny` gives us what we want as far as filtering items that would be valid for implicit conversions goes. Also, we concluded that the `All` option was also not acceptable as only valid extensions must be enforced in the collection creator.
Place it right next to the buttons for choosing between batch or singular collection
- `buildCollectionModal.ts` still exists but just to create the rules collection modal, all other modals are replaced with a parent `CollectionCreatorModal` - also added a `collectionBuilderItemsStore` that is used to store the datasets fetched for the builder; only when the builder is opened: which is when we start reacting to any changes in the `historyId`, `history.update_time` and any filter on a history selection.
`stringifyObject` does not need to be re-run for every selected value. Co-authored-by: Laila Los <[email protected]>
In the case of `list`, they are now added directly to the final list. In the case of `list:paired` and `paired`, they are added to the `workingElements`; the user can then manually add them. This is all still done within the `CollectionCreator`. Next thing to look at is to try to only allow/wait for those uploaded files to be attempted as collection additions if they are of `ok` state. One way to do that is to maybe use a list to keep track of uploaded items in the creator, and if those uploaded items disappear from `workingElements` (meaning they are invalid) we have explicit Toasts mentioning this.
…ntent variability
fix selector for name collection in collection builder fix hide original items selector in collection creator fix clear filters selector in collection creator
5090d7b
to
03527bd
Compare
This fixes seleniums failing in `lib/galaxy_test/selenium/test_library_to_collections.py::TestLibraryToCollections` where the modal was in the way.
Major action items (UPDATE 11/19 now complete for this PR):
✅ Done: If there are required extension(s), we filter out the items with the required extension (but we also include any items that have extension that
isSubTypeOfAny
; meaning it can be implicitly converted to become a valid input).list:paired
, do not attempt to auto pair by default and simply show all items.In case the current history is empty and to make it clearer in general, allow history to be switched from within the modal?Seems like a bad idea, history can just be switched from the Switch history modal; and in the case of a history-less (singular history) UI, this won't be a concern either✅ Done: The collection creator (for all types:
list
,paired
,list:paired
) allows users to upload datasets (even binding those to the required extension(s)) directly within itself, and even adds them to the resulting list (only in the case oflist
, for the rest they are shown in the updated available datasets to add list).Note:Done ✅paired
collection is still a WIPFixes #18704
guide_users_to_collection_builder.mp4
Future improvements incoming:
Maybe we can merge this for the 24.2 release and then I can build upon these changes with the following enhancements:
list
,list:paired
andpaired
are supported/improved by these changes. It would be cool if users can create a shape likelist:list:paired
on the fly. Please let me know if this is a good idea and/or what are other possibilities that can empower users further here.TODO
s here and there right in the code changes in this PR which would improve UX and the code itselfOutdated screencast
guide_users_to_collection_builders_wip_2.gif.mp4
How to test the changes?
(Select all options that apply)
License