Problem/Motivation

The current export links are rendered as feed icons on the bottom left of the view. This makes them hard to discover. It would be better for us to follow Drupal best practices and create local actions for actions related to the current view.

Proposed resolution

Add local actions instead of feed icons.

Screenshot

Remaining tasks

/

CommentFileSizeAuthor
#18 composer.json — workspace 2024-02-02 11-08-32.png35.39 KBrealityloop

Issue fork views_data_export-3414095

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DieterHolvoet created an issue. See original summary.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented

I have written a working prototype that adds local actions instead of feed buttons, but there's one issue: if the view has AJAX enabled and exposed filters are changed, the URL in the local action is not updated, which causes filters to not be passed to the export.

A couple possible solutions:

  • force AJAX to be disabled if we want to add a local action (not ideal)
  • make sure the local action region is rebuilt using AJAX when the view is rebuilt using AJAX (sounds complicated)
  • write some custom JS updating the local action query params when the current page query params change (possible too hacky?)
DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented
Status: Active » Needs work

More (possible) todo's:

  • Make sure local actions are regenerated when export display handlers are added/edited/deleted
  • Add a new setting to show the local action, since it's no longer part of the feed icons

DieterHolvoet opened merge request !31

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented
Related issues: +#3397914: Multiple export download links should be distinguishable

I changed the local action name to use the display title, to fix the issue as described in #3397914: Multiple export download links should be distinguishable.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented

I fixed the issue where the local action title is not updated if the display title changes. Pretty sure the local action cache still needs to be invalidated when a new view is created, in order for the new local action to be generated, but I haven't tested this yet.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented

When removing the old feed icons I discovered we'll have to re-implement the auto download functionality - not sure what exactly it does though.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented
Issue summary: View changes
DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented
Issue summary: View changes

I added some JS that keeps the local action query params up to date with the current page's query params.

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented

If I remember correctly, the auto-download works using a CSS selector in the message shown with the download link. JavaScript selects the link from the message and downloads the file.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented
Issue summary: View changes
DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented

It looks like the 'Include query string parameters on redirect' option currently only applies to batched exports. By updating the query params in JS, it will also work for direct exports.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented

Never mind, the redirect functionality is obviously not available for direct downloads.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented
Issue summary: View changes
DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented
Issue summary: View changes
Status: Needs work » Needs review
solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented
Status: Needs review » Needs work

There is a failing test.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented
Status: Needs work » Needs review
realityloop’s picture

realityloop
he/him
CreditAttribution: realityloop as a volunteer and at Realityloop commented
FileSize
composer.json — workspace 2024-02-02 11-08-32.png35.39 KB

I don't know why but this diff will not apply..

    https://git.drupalcode.org/project/views_data_export/-/merge_requests/31.diff ()
   Could not apply patch! Skipping. The error was: Cannot apply patch https://git.drupalcode.org/project/views_data_export/-/merge_requests/31.diff

    https://www.drupal.org/files/issues/2023-02-20/views_data_export-3173296-25.patch (https://www.drupal.org/project/views_data_export/issues/3173296#comment-14930767)
    ./patch/31.patch ()
   Could not apply patch! Skipping. The error was: Cannot apply patch ./patch/31.patch

realityloop’s picture

realityloop
he/him
CreditAttribution: realityloop as a volunteer and at Realityloop commented
Status: Needs review » Needs work
DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented
Status: Needs work » Needs review

MR's are based on the latest dev commit, not the latest tagged release. You can use the dev release (composer require drupal/views_data_export:dev-1.x) or you can try to rebase the branch against the latest tagged release and create a patch from that.

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented
Status: Needs review » Needs work

I'm not able to get the local download action to show. I can't figure out how to add the block to my page layout.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented
Status: Needs work » Needs review

I'm sorry, but that sounds like an issue with your website, not with this MR. This MR shows local tasks just like core and every other contrib module does.

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented

I disagree. I have local tasks already showing, but your added tasks do not show. That in itself shows that this would be a breaking change

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented

I installed the dev version, applied the patch, cleared the cache, and yet I do not see any "Download CSV export" task link on the page where previously the Download CSV button showed. I am still able to see the standard View, Edit, Delete, Layout, and Revisions local tasks.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented

Oh okay, so the issue is not about adding a block to your page layout. The local tasks of this issue should show in the same place as other local tasks do. Is the problem fixed after you clear caches? Did you change the 'Attach to' setting to include the display you want the local task to appear on?

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented

Yes, the "attach to" option is correctly set. No, the download option doesn't show after clearing the cache.

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented

Ah, I think the problem is that you added the link as an action, rather than as a task. I don't use the actions module.

This solution/change won't work for anyone who doesn't use actions.

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented
Status: Needs review » Needs work

I think this should be thought through. If you look at examples in Core/Contrib, most action links are "Add X". In fact, I have yet to find one that isn't an "Add".

  • Comment: Add comment type
  • Key: Add Key, Add Override
  • Node: Add content type, Add content
  • Path: Add alias
  • System: Add format
  • Update: Add new module or theme, Add new module, Add new theme
  • User: Add user, Add role

All the other local-type links are tasks.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented

Right, good catch. I still believe it makes sense to keep it a local action since downloading is an action: it doesn't even link to a separate page, it just downloads something. Local tasks are shown as tabs, they are links to separate pages. It would be confusing if clicking a tab wouldn't navigate to another page, but instead would download a file. Here's some info about local actions vs local tasks (tabs).

By the way, you don't need the actions module to be able to show local actions. Local actions are a core feature that can't be disabled. You might need to place the local actions block if you haven't yet, but I can't imagine a lot of people aren't using that block. It places useful links all throughout the admin interface.

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented

I have the block placed... in the admin theme. The download action on views is not going to be in the admin theme for most views with exposed filters, at least not in my case. My views with data export are on the user-facing non-admin part of the site.

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented

Okay, so how do you propose going forward? Should we give the option to either show the old icon or the new local action? Or do you have something else in mind?

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented

I think if we have a config setting to either use a local action or a download button, that would work well.

Though I think it'd be better to add in the solution from #3379233: Increase size of download buttons instead of using the prior images. Images don't go well with site themes, and making them buttons works well.