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

Add download dropdown lists to dataTables #1593

Merged
merged 4 commits into from
Jul 26, 2023
Merged

Add download dropdown lists to dataTables #1593

merged 4 commits into from
Jul 26, 2023

Conversation

lfarrell
Copy link
Contributor

Copy link
Member

@bbpennel bbpennel left a comment

Choose a reason for hiding this comment

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

Looks good over all, left some suggestions and questions

});
},

downloadButtonHtml() {
Copy link
Member

Choose a reason for hiding this comment

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

I am guessing this had to be duplicated because the html of the datatable is dynamically generated, so there wasn't a place to add in the fileDownload.vue component? It looks like we do have access to the current vue component from inside of the datatable though, so we may be able to cut down on duplication.

One option would be to replace fileDownload.vue with the fileDownloadUtils mixin, using this method (receiving the brief_object as a param) to generate the html instead of a template everywhere. Then I think we could call downloadButtonHtml where ever we need that element. Preferably the mixin could include the dropdown closing logic too.

It might also be possible to dynamically add another component via generated html being added to the page, either by some programmatic means of getting vue to import a component at a particular place in the html, or maybe even just including the <fileDownload></fileDownload> tag in the generated HTML if its going to be added to the page inside of the area controlled by vue. My impression is that vuejs is supposed to evaluate insertions to the HTML in these scenarios, but I'm not totally sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The datatable component does its own thing with the DOM. I'm pretty sure I tried just adding <fileDownload/> and it just prints out <fileDownload/> as an HTML tag.

Copy link
Member

Choose a reason for hiding this comment

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

What about using downloadButtonHtml instead of fileDownload.vue everywhere? So that we would just have one implementation of it.

static/js/vue-cdr-access/src/mixins/fileDownloadUtils.js Outdated Show resolved Hide resolved
@bbpennel bbpennel merged commit 1869e50 into main Jul 26, 2023
2 checks passed
@bbpennel bbpennel deleted the work-download branch July 26, 2023 19:44
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.

2 participants