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

Render arbitrarily nested lists in the UI #5038

Merged
merged 25 commits into from Nov 30, 2017

Conversation

Projects
None yet
4 participants
@jmchilton
Member

jmchilton commented Nov 20, 2017

I started with #5013 and simply refactored the client until I eliminated 20 classes (Backbone models and views) and many places collection type was dispatched on to produce different classes without different behavior. This resulted in no features lost but now arbitrarily nested lists render properly.

The classes lost in this refactoring include:

  • ListDatasetCollection
  • PairDatasetCollection
  • NestedPairDCDCE
  • NestedPairDCDCECollection
  • ListPairedDatasetCollection
  • NestedListDCDCE
  • NestedListDCDCECollection
  • ListOfListsDatasetCollection
  • ListCollectionViewEdit
  • PairCollectionViewEdit
  • ListOfPairsCollectionViewEdit
  • ListOfListsCollectionViewEdit
  • ListCollectionView
  • PairCollectionView
  • ListOfPairsCollectionView
  • ListOfListsCollectionView
  • HistoryPairDatasetCollection
  • HistoryListPairedDatasetCollection
  • HistoryListOfListsDatasetCollection
  • NestedCollectionElementViewEdit

The exports for various modules this touches reveal how this simplified the interface:

export default {
- ListDatasetCollection: ListDatasetCollection,
- PairDatasetCollection: PairDatasetCollection,
- ListPairedDatasetCollection: ListPairedDatasetCollection,
- ListOfListsDatasetCollection: ListOfListsDatasetCollection
+ DatasetCollection: DatasetCollection,
};
 export default {
     CollectionViewEdit: CollectionViewEdit,
-    ListCollectionViewEdit: ListCollectionViewEdit,
-    PairCollectionViewEdit: PairCollectionViewEdit,
-    ListOfPairsCollectionViewEdit: ListOfPairsCollectionViewEdit,
-    ListOfListsCollectionViewEdit: ListOfListsCollectionViewEdit
 };
 export default {
     CollectionView: CollectionView,
-    ListCollectionView: ListCollectionView,
-    PairCollectionView: PairCollectionView,
-    ListOfPairsCollectionView: ListOfPairsCollectionView,
-    ListOfListsCollectionView: ListOfListsCollectionView
 };
 export default {
-    HistoryListDatasetCollection: HistoryListDatasetCollection,
-    HistoryPairDatasetCollection: HistoryPairDatasetCollection,
-    HistoryListPairedDatasetCollection: HistoryListPairedDatasetCollection,
-    HistoryListOfListsDatasetCollection: HistoryListOfListsDatasetCollection
+    HistoryDatasetCollection: HistoryDatasetCollection,
 };

This also builds on the collection state tests from #5013 with many more collection tests - testing the rendering of various kinds of collections and some collection history operations (e.g. nametags).

@dannon

This comment has been minimized.

Member

dannon commented Nov 20, 2017

Haven't tested this yet, but thank you for flattening this.

@jmchilton

This comment has been minimized.

Member

jmchilton commented Nov 20, 2017

That test failing Selenium test passes pretty consistently locally and fails pretty consistently on Jenkins - huh. How odd - I'll mark this as WIP.

@martenson martenson changed the title from Render arbitrarily nested lists on the UX. to Render arbitrarily nested lists in the UI Nov 20, 2017

@jmchilton jmchilton added status/review and removed status/WIP labels Nov 29, 2017

jmchilton added some commits Nov 16, 2017

Refactor client collections - collapse List and Paired models.
They don't have different functionality - they shouldn't be different.
Refactor client collections - rename List -> Flat at lowest level mod…
…els.

It represents both lists and paireds now.
Refactor client collections - renamed list collection edit to flat.
Since it represents boths lists and paireds now.
Refactor client collections - rename nested collection views.
Both edit and RO variants - to reflect new generality.
Refactor client collections - rename NestedPairCollectionViewEdit
... to NestedCollectionElementViewEdit since it isn't just used for paired collection contents and hasn't been for some time.
Refactor client collections - eliminate Flat vs Nested dataset collec…
…tion.

Just dispatch on collection type to determine collection contents.
Refactor client collections - eliminate FlatCollectionView & FlatColl…
…ectionViewEdit.

These just did the same thing as CollectionView and CollectionViewEdit.
@@ -398,18 +398,18 @@ var NestedListDCDCECollection = NestedDCDCECollection.extend({
//==============================================================================
/** @class Backbone Model for a DatasetCollection (list) that contains other lists. */
var ListOfListsDatasetCollection = DatasetCollection.extend({
var NestedDatasetCollection = DatasetCollection.extend({

This comment has been minimized.

@martenson

martenson Nov 30, 2017

Member

This makes so much sense.

@martenson martenson merged commit 0c97e11 into galaxyproject:dev Nov 30, 2017

5 of 7 checks passed

api test Build finished. No test results found.
Details
selenium test Build finished. No test results found.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 164 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 59 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment