- 🇺🇸United States jhedstrom Portland, OR
I think 🐛 jQuery.once used in views_data_export_auto_download.js, breaking immediate download for batch exports Fixed is somewhat related for D10+.
The patch #8 → from #3347083 🐛 jQuery.once used in views_data_export_auto_download.js, breaking immediate download for batch exports Fixed fixed the issue (tested on 8.x-1.2 and Drupal 10).
I tried #17, and it doesn't really do anything to help. It does work like #14 scenario 4 described, but that already worked.
It doesn't fix any of the scenarios that don't work, as the other scenarios still don't have automatic download. The JS should be included everywhere if needed, and there's plenty of contrib modules that include a library on every page.
https://www.drupal.org/docs/develop/creating-modules/adding-assets-css-j... →
Also, the only way to attach JS to a message would be to modify the twig template, and it would be for all messages: https://api.drupal.org/api/drupal/core%21modules%21system%21templates%21...
So just including on every page is better.
- last update
about 2 years ago 3 pass - @solideogloria opened merge request.
Please review the merge request, which fixes the issue for all the scenarios mentioned in #15.
- last update
about 2 years ago Composer require-dev failure - last update
about 2 years ago 3 pass Here's a patch file with the changes from #22 if you need it
- 🇫🇷France PhilY 🇪🇺🇫🇷 Paris, France
Patch #22/#24 works for me using Scenario 1, views_data_export 8.x-1.3 (with xls_serialization addon) and Drupal 9.5.9
Thanks - Status changed to RTBC
about 2 years ago 2:43pm 29 June 2023 - last update
over 1 year ago 3 pass - last update
over 1 year ago 3 pass Hi friends! When I check the available translations, I come across the following:
In my case, when I download the data from a View, I encounter the following message:
"Export complete. Download the file false" id="vde-automatic-download">here if the file is not automatically downloaded."
Upon inspecting the 'views_data_export_auto_download.js' file, I noticed that the file only downloads automatically if the 'downloadEnabled' attribute is set to 'true'.
To address this issue, I made a translation change and modified the message to:
"Export complete. Download the file true" id="vde-automatic-download">here if the file is not automatically downloaded."
Additionally, I included the line $view->element['#attached']['library'][] = 'views_data_export/views_data_export'; in the view to invoke the necessary library.
@Sebastian45 That change was included in patch #24. Did you try using a patch?
- 🇷🇸Serbia miksha
For the batch process issue that is explained in #15 I am suggesting another approach that doesn't require us to include JS in all site pages (which in turn also requires core.once).
We are using status message (visible to both anonymous and authenticated user) to show link to exported file and we can use this message on the redirected page to dynamically load our library only on required pages.
I am attaching a patch for this. I tested it locally and works for both anonymous and authenticated users.
@miksha The class/attribute used should be specific to VDE, not just every message that has
data-download-enabled="true"
, because other modules could have that.- Status changed to Needs review
10 months ago 12:46am 9 September 2024 - Status changed to Needs work
8 months ago 2:58pm 18 November 2024 Using status messages doesn't work if the site doesn't use that block to show messages. I think that including the JS on every page is fine because the JS is small and doesn't really affect the load time that I can tell.
I think #22 (aka #24) should be used.
I dont think the approach in the patches are exactly right. The page_attachments_alter approach in #24 would attach the library on every page on the site. #29/31 relies on checking the status messages array which seems unnecessary and prone to breaking as explained in #33.
This patch here I believe is appropriate. It will attach the library on any view that uses the data_export display.
if (str_starts_with($display_id, 'data_export')) {
This is not ideal. Users can set custom machine names for displays, can't they?
See /admin/structure/views/nojs/display/VIEW_ID/DISPLAY_ID/display_id
Hmm yeah and the more I think about it a hook in general doesnt seem like a great solution.
To recap though, all of this is because the library is attached with the icon, but a lot of sites don't want to use the icon because it's not that easy to style/position. I think what should be done is keeping the config option to attach the export to a view, but separating it from the icon by creating a separate option to attach an icon to the view. This way, you can choose to attach the library to a specific view, without needing to necessarily use the icon provided by the module, though that option is still available.
- 🇷🇸Serbia miksha
I suggest another approach where we add query parameter to the redirect url, and this way we do not rely on status messages, views display names etc. I attach new patch.
I was having trouble getting the functionality in the patch #37 to work, but also, I still dont think this should be a hook. I like how the current implementation attaches the library only when the data_export display is attached to the view. The MR I opened keeps that functionality, but allows the user to separately decide if they also want the icon to display.
- 🇷🇸Serbia miksha
@mdranove I believe we have two situations to handle: one that you covered and one that I covered. In my case I needed to fix the case where we use batch mode of exporting data and then use redirect to a certain page when page is done. When we redirect to a custom page we don't have this library attached to that custom page. So, my solution in #37 was to pass query parameter to the redirect url, and then use hook to get query parameters and attach library only if parameter is correct.
@miksha, good point, you're right, that being said, in #37 I get a 403 error when I try it. I do think that might be the way to go, although I wonder if there isn't a way to leverage the "include query parameters" config that already exists.
- 🇷🇸Serbia miksha
@mdranove Thanks for checking. I did test it locally, but will re-test using my patch in #37 on a clean install, to be sure I didn't mess up a patch. Regarding "include query params", they are optional and in my opinion functionality of a module should be there even without user interaction. Also leaving it to the users would be prone to errors. And download of exported data SHOULD ALWAYS happen when batch/redirect is selected. I don't see it as an optional thing. As soon as I get some free time I will get back to you.