[WIP] 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 |
| @@ -39,4 +39,14 @@ export class DatatypesMapperModel { | |||
| isSubTypeOfAny(child: string, parents: DatatypesCombinedMap["datatypes"]): boolean { | |||
| return parents.some((parent) => this.isSubType(child, parent)); | |||
| } | |||
|
|
|||
| /** For classes like `galaxy.datatypes.{parent}.{extension}`, get the extension's parent */ | |||
| getParentDatatype(extension: string) { | |||
There was a problem hiding this comment.
That logic is redundant and also doesn't reflect how datatypes match. Is there a problem with using isSubTypeOfAny ?
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 |
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
ahmedhamidawan
force-pushed
the
guide_users_to_collection_builder
branch
from
7d474c2
to
12f6957
Compare
client/src/components/Form/Elements/FormSelectMany/worker/selectManyMain.ts
Outdated
Show resolved
Hide resolved
- `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]>
Major action items:
✅ 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 eitherNote:
pairedcollection is still a WIPFixes #18704
guide_users_to_collection_builders_wip_2.gif.mp4
How to test the changes?
(Select all options that apply)
License