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

Vue-ify Upload Dialog w/small fixes #9167

Merged
merged 25 commits into from
Jan 29, 2020
Merged

Conversation

jmchilton
Copy link
Member

@jmchilton jmchilton commented Dec 27, 2019

See individual commits for details.

  • Removed upload-button.js - mostly unused after Porting Tools Panel to Vue.js (resolves #8262) #8368.
  • Simplifications and ES6 stuff to existing upload Backbone components.
  • Swap upload-view to UploadModal and use b-modal instead of Galaxy's modal stuff - use no-enforce-focus to ensure Select2 works within the modal.
  • Migrate usage of upload Tabs to b-tabs - (initial shim for Backbone views in early commits replaced with Vue components for each tab type in subsequent commits).
  • Replace each mvn/upload/<type>/<type>-view.js Backbone View with a Vue component.
    • Migrate composite-view.js to a new Composite.vue - eliminates Backbone view for everything but the row. Still relies of Backbone models.
    • Migrate collection-view.js to a new Collection.vue - eliminates Backbone view for everything but the row. Still relies of Backbone models.
    • Migrate rules-input-view.js to a new RulesInput.vue - eliminates Backbone view, never used row views or models.
    • Migrate default-view.js to a new Default.vue - eliminates Backbone view, eliminates Backbone view for everything but the row rending. Still relies on Backbone models.
    • More reuse than Backbone code given mixins and components - this PR vastly reduces the duplication between the collection and default uploads.
  • Parameterized lazy limited widget handling between Default.vue and Collection.vue - either can have a lazy limited set via a Vue property. Really reduces the divergence between default upload and collection upload nicely. I added some tests for this and fixed an off by one error inside of lazy-limited.js.
  • Largely unrelated to Vue, but I took a first crack at eliminating a lot of the copy-paste duplication between the Backbone views in default-row.js and collection-row.js. Just cleaning up past messes I made ahead of potentially Vue-ifying these as well.
  • Eliminates some qunit tests, adds more modern vuejs-utils tests. Not exactly one-to-one because some jquery plugins behave different in karma vs. qunit but definitely considerably more tests now.
  • Generalize special case handling of FTP files that uploads them all in a single request for default uploads to work with collection uploads also - removing divergence between two upload modalities.

@jmchilton jmchilton added kind/enhancement area/UI-UX kind/refactoring cleanup or refactoring of existing code, no functional changes area/upload labels Dec 27, 2019
@jmchilton
Copy link
Member Author

jmchilton commented Dec 27, 2019

Continued the vue-ification throwing in the tabs and the actual composite uploader view... select2 still not working. I think the way the tabs work - this is all now creating the select2 components later... I had hope that would fix things. (Fixed see comment below)

@jmchilton
Copy link
Member Author

jmchilton commented Dec 27, 2019

It was the tabindex on b-modal... select2/select2-bootstrap-theme#41

https://bootstrap-vue.js.org/docs/components/modal/ no-enforce-focus on the modal seems to fix it - but it is strongly discouraged. I guess it is no worse than our current modals?

@jmchilton jmchilton force-pushed the upload_vue_0 branch 6 times, most recently from 75bc4c5 to 9d95adb Compare December 30, 2019 15:10
@jmchilton jmchilton force-pushed the upload_vue_0 branch 2 times, most recently from 315d970 to e44be77 Compare January 3, 2020 22:04
@jmchilton
Copy link
Member Author

@galaxybot test this

@jmchilton jmchilton force-pushed the upload_vue_0 branch 4 times, most recently from 9968178 to afb74f4 Compare January 13, 2020 15:48
@jmchilton jmchilton changed the title [WIP] Work toward Vue-ifying Upload Dialog Vue-ify Upload Dialog w/small fixes Jan 13, 2020
@galaxybot galaxybot added this to the 20.05 milestone Jan 13, 2020
@guerler
Copy link
Contributor

guerler commented Jan 22, 2020

Noticed that the select2 dropdown width seems to be fixed to the actual parent dom width:

widthselect2

Fill out Select2.js as part of that.
Rework a lot of Collection.vue into shared mixin. There was a lot of duplication in there (all my fault).
- Let the components just think about this abstractly and move the logic into the common base class.
- Add tests!
@jmchilton
Copy link
Member Author

I've fixed the Select2 width issue with 224a92a I believe. Thanks for the catch @guerler

@guerler
Copy link
Contributor

guerler commented Jan 22, 2020

I think we need to vue-ify the backbone rows too, other than that this looks great.

@jmchilton
Copy link
Member Author

I think we need to vue-ify the backbone rows too

Eventually that would make sense, but there is no reason that would have to happen as part of this PR I assume. I did this to enable #9110 and #9114 not to just to Vue for the sake of Vue-ing.

@dannon
Copy link
Member

dannon commented Jan 29, 2020

This seems fine as an incremental step. As we eliminate the rest of the backbone (especially the models..) it'll all be a lot cleaner.

Thanks!

@dannon dannon merged commit 79bb7fd into galaxyproject:dev Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/UI-UX area/upload kind/enhancement kind/refactoring cleanup or refactoring of existing code, no functional changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants