Update pagination icons to use SVGs

[Scotchester]

Updates pagination left and right arrow icons to use SVG icons.

I couldn't figure out how to invoke the icon template inclusion tag from inside the pagination_link_previous and pagination_link_next tags defined in modeladmin_tags.py, so I settled for just rendering the icon.html template into a variable. Definitely open to hearing better ideas on this!

The other mild concern I had was the fact that there are three templates that create very very similar pagination code. I didn't have time to fully digest the differences, but I see in the release notes for 2.5 that it was definitely a conscious decision to split out ajax_pagination_nav from pagination_nav. What I'm really not sure about is why _pagination and pagination_nav both exist. The main difference appears to be how the URLs for the pagination links are constructed, but I'm not sure why they are different or if they need to be. @gasman, looks like you're the primary contributor to both files. Would you have any insight into that?

---

Thanks for contributing to Wagtail! 🎉

Before submitting, please review the contributor guidelines https://docs.wagtail.io/en/latest/contributing/index.html and check the following:

• Do the tests still pass? (https://docs.wagtail.io/en/latest/contributing/developing.html#testing)
  • There's a failing Draftail Jest snapshot, but I don't believe that would have anything to do with what I've done here
• Does the code comply with the style guide? (Run make lint from the Wagtail root)
• For Python changes: Have you added tests to cover the new/fixed behaviour?
  • N/A
• For front-end changes: Did you test on all of Wagtail’s supported browsers? Please list the exact versions you tested.
  • No, but should work the same as other SVG icon changes
• For new features: Has the documentation been updated accordingly?
  • N/A

[squash-labs]

Manage this branch in Squash

Test this branch here: https://pagination-icons-wrec0.squash.io

[gasman]

@gasman, looks like you're the primary contributor to both files. Would you have any insight into that?

From what I can remember, before I split it up into multiple templates it was a nasty hodgepodge of {% if %} / {% else %} cases with not much in common, so it was really one template doing several different jobs that looked superficially similar. I suspect I was in the middle of some wider template refactoring and didn't want to get sidetracked into rewriting the view code to make everything follow the same pagination logic - as far as I'm concerned there's no fundamental reason to keep these separated, though, and I'd be very happy to see this cleaned up further!

[Scotchester]

@gasman, looks like you're the primary contributor to both files. Would you have any insight into that?

From what I can remember, before I split it up into multiple templates it was a nasty hodgepodge of {% if %} / {% else %} cases with not much in common, so it was really one template doing several different jobs that looked superficially similar. I suspect I was in the middle of some wider template refactoring and didn't want to get sidetracked into rewriting the view code to make everything follow the same pagination logic - as far as I'm concerned there's no fundamental reason to keep these separated, though, and I'd be very happy to see this cleaned up further!

Thanks for that! I'll see if Thibaud and/or Coen think it's worth trying to do that here, or if we should just keep the scope restricted to the icon changes.

[thibaudcolas]

This looks simple to review, should we try to get this in as it is and you do the refactoring in a separate PR? Or would the refactoring drastically change how the icons are defined?

[Scotchester]

@thibaudcolas Yeah, let's hold the refactoring for another PR. It wouldn't change the icon updates, really.

[thibaudcolas]

This looks good to me, with the caveat that the icons are much bigger than before – I think I like it like that, but just wanted to confirm it’s intentional and we’re happy for it to stay.

Before/after:

Tested in IE11 and Chrome, with the image listings, image chooser, and modeladmin listings.

[thibaudcolas]

I’m not a big fan of this default class name (default what?), but this was here before so oh well.

[Scotchester]

Yes, the larger size was intentional and I forgot to comment on it! Sorry about that. Glad you like it :)

Yeah, the default class name is not great, but it's already in use to give SVG icons a "default" size that is actually more reasonable than what the SVG would produce on its own (which is very large), so I used that here, too.