Easy-digital-downloads: Reposition/refactor the pagination in the [downloads] shortcode

Created on 29 Jul 2018  路  13Comments  路  Source: easydigitaldownloads/easy-digital-downloads

The pagination (#edd_download_pagination) in the [downloads] shortcode is currently a sibling of each download in the .edd_downloads_list div.

I propose:

We move the pagination so it appears after .edd_downloads_list in the HTML markup.

Doing so will fix a fundamental problem when trying to use flexbox or CSS grid to lay out the downloads. Since the pagination is a sibling of the actual downloads, it takes on the properties on display: flex or display: grid when they have been applied to the parent container (.edd_downloads_list).

Here's an example where the pagination is positioned incorrectly (it should be below the downloads). The reason why it does this is because it has essentially become a flex item:

screen shot 2018-07-29 at 7 14 23 pm

The above is a very basic example which can easily be solved by making the pagination div 100% wide however more CSS shouldn't be needed. Depending on the grid/flex properties used, more CSS may also be needed to fix the positioning/look of the pagination.

The pagination is not a download so it should not exist within the div that wraps each download.

We move the pagination to its own dedicated function.

The downloads shortcode is huge, moving the pagination to its own function makes things easier to see and manage.

In the near future we should also build a dedicated function for outputting a grid of downloads. This is a small step towards that goal and would mean themes and other plugins can build download grids without having to call the shortcode. And of course the [downloads] shortcode would pass its attributes to this function to build its own download grid.

The downloads block in EDD Blocks as an example currently needs to use the [downloads] shortcode as there is no other way to build a download grid: https://github.com/easydigitaldownloads/edd-blocks/blob/master/includes/blocks/downloads/index.php#L33-L38.

We load the dedicated function on the edd_downloads_list_after action hook.

This would allow themes/plugins to unhook the pagination and completely customize it (something I need to be able to do). The pagination's existing filter can only get you so far.

Other notes

  1. The current edd_downloads_list_after action hook doesn't actually work correctly. Because of where it's positioned in the code + object buffering, anything echoed to the hook will be shown ABOVE the download grid. The proposed PR fixes this.

  2. Since the pagination is now moved, we can reduce our core CSS every so slightly. The #edd_download_pagination CSS rule can be safely removed since the pagination is no longer affected by CSS floats.

  3. This gets us one step closer to migrating to a grid layout using flex or grid CSS.

  4. It should look exactly the same as it did before on the majority of themes, provided the theme has not styled the download grid a different color. I noticed this with Vendd. Since the download grid is grey, and the pagination is now moved outside the div, the pagination now has a white background. It will need a small CSS fix.

type-bug type-feature

All 13 comments

This actually affects Vendd even less than you mentioned above. If the EDD Downloads template is used for the [downloads] shortcode, there's no change at all. The background issue you mentioned probably only happens if you're using the [downloads] shortcode in regular content. Still a valid observation, though.

Testing.

Love this, by the way. So tired of downloads shortcode hacks because pagination is a sibling to the download items. This is exactly why I stopped here in Vendd.

Going to punt until we get the refactor done for this.

This, along with 6852, are waiting on a single function for pagination, right?

@SDavisMedia Correct, I'll work on the pagination today and get it ready for testing.

@SDavisMedia @cklosowski Updated the PR and left a note here: https://github.com/easydigitaldownloads/easy-digital-downloads/pull/6809#issuecomment-419773931

Testing now. Gonna report any issues as I go.

I originally went through a round of testing for this on the day I left my last comment. Most everything was working well. 馃憣 Today I pulled down the latest issue/6808 and loaded a page where I use the [downloads] shortcode, also set as the site Front Page.

Just on the load, I'm met with an error that kills the entire shortcode output:

[12-Sep-2018 16:05:22 UTC] PHP Fatal error:  Uncaught ArgumentCountError: Too few arguments to function edd_downloads_pagination(), 2 passed in /app/public/wp-includes/class-wp-hook.php on line 286 and exactly 3 expected in /app/public/wp-content/plugins/easy-digital-downloads/includes/template-functions.php:1152
Stack trace:
#0 /app/public/wp-includes/class-wp-hook.php(286): edd_downloads_pagination(Array, Object(WP_Query))
#1 /app/public/wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters('', Array)
#2 /app/public/wp-includes/plugin.php(465): WP_Hook->do_action(Array)
#3 /app/public/wp-content/plugins/easy-digital-downloads/includes/shortcodes.php(653): do_action('edd_downloads_l...', Array, Object(WP_Query))
#4 /app/public/wp-includes/shortcodes.php(325): edd_downloads_query(Array, '', 'downloads')
#5 [internal function]: do_shortcode_tag(Array)
#6 /app/public/wp-includes/shortcodes.php(199): preg_replace_callback('/\\[(\\[?)(downlo...', 'do_shortcode_ta...', '[downloads pric...')
#7 /app/public/wp-includes/class-wp-hook.php(286): do_shortcode in /app/public/wp-content/plugins/easy-digital-downloads/includes/template-functions.php on line 1152

Still testing.

Note that after a hard refresh, the error went away. And my [downloads] shortcode actually didn't use pagination when the page initially loaded. I had all products displaying without the need for pagination.

I just adjusted my shortcode to use pagination and it's working with no errors. So maybe that was a fluke... not sure.

Still testing.

Everything is looking really good to me! Can't break it at the moment. Super nice.

This is working great @amdrew . Appreciate the work to get that broken out into it's own functions. I'm going to actually write up some unit tests to see if I can produce some confirmed states with it now.

Sweet as 馃憤

Great, so it looks like WP core changed their pagination output in the 4.9 releases. Going to have to write in some way to account for that in the unit tests.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

davidsherlock picture davidsherlock  路  5Comments

zackkatz picture zackkatz  路  5Comments

JeroenSormani picture JeroenSormani  路  5Comments

SDavisMedia picture SDavisMedia  路  3Comments

michaelbeil picture michaelbeil  路  5Comments