Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
It seems that view "tags" options (link titles - "next", "last"...) don't pass to theme_views_mini_pager (views/theme/theme.inc). it works for full pager, but not for mini pager.
Comment | File | Size | Author |
---|---|---|---|
#55 | drupal-1625248-55.patch | 31.75 KB | dawehner |
#48 | drupal-1625248-48.patch | 31.75 KB | dawehner |
#41 | drupal-1625248-41.patch | 32.23 KB | dawehner |
#39 | drupal-1625248-39.patch | 41.49 KB | dawehner |
#37 | drupal-1625248-37.patch | 32.23 KB | dawehner |
Comments
Comment #1
pschuelke CreditAttribution: pschuelke commentedI get the same error/issue. The mini pager tags->text for links do not change the display. Also if you use a '-' it gives a 'Warning: Invalid argument supplied for foreach() in views_object->unpack_translatable() (line 305...' You can get around it here.
Comment #2
walker2238 CreditAttribution: walker2238 commentedComment #3
Jorrit CreditAttribution: Jorrit commentedPlease see the attached patch for a fix.
Comment #4
dawehnerThe only problem i have with this patch at the moment is that it will probably change the current output
of mini pagers.
The defaults defined on the full pager are "« first" etc. so they will be also rendered like that.
You probably have to change the default values in options_definition as well.
Comment #5
Jorrit CreditAttribution: Jorrit commentedThanks for your remarks, you're right. I have found another small issue: the
views_theme()
definition andtheme_views_form_views_form()
expect aquantity
variable, but that variable isn't actually used nor supplied. So my patch also removes that. I am interested to hear your comments on my new patch.Comment #6
dawehnerWe could add a // @see reference for that.
Oh is this variable not used at all?
Comment #7
Jorrit CreditAttribution: Jorrit commentedYes.
theme_views_pager_mini
is a copy of corestheme_pager
, but not everything that should have been removed, was removed.Comment #8
dawehnerThank you for this clean solution!
Oh i see, so i think this is RTBC after a small // @see
Comment #9
Jorrit CreditAttribution: Jorrit commentedHere you go. I guess versions 6.x-3.x and 8.x-3.x also needs an update, I'll look at that later.
Comment #10
dawehnerWhat about using Overrides views_plugin_pager_full::$function instead, but keep the extra comments?
The rest of the patch is certainly RTBC. No worries about d8, we can port this after the patch got in, even the othe way round makes us a bit happier.
Comment #11
Jorrit CreditAttribution: Jorrit commentedIs that a Drupal convention? I didn't know about that. Here is a new patch.
I can't get Drupal 8 to work at this moment, perhaps the core 8.x branch is broken temporarily. I'll check later.
Comment #12
dawehnerYeah, the handbook page http://drupal.org/node/1354 is pretty handy for that, i just use that one handbook page all the time :)
Tests maybe not be required for 7.x but at least for 8.x we really have to be sure that everything works (especially things which have been broken before), so adding tags. If you don't have the endurance to work on tests/d8 i will for sure do that.
Comment #13
Jorrit CreditAttribution: Jorrit commentedI accidently saw your checkin at #1805674: Mini pagers should extend full pagers. I was already puzzled that the mini pager didn't extend the full pager in D8, thanks for fixing that. I still don't have Drupal 8 running, though.
Do you have any idea when the patch can be committed? Do other people need to take a look at it?
Comment #14
xjmLet's fix this in 8.x first since it affects #1070166: As a screen-reader user I need a way to recognize the purpose of the Next and Previous links in the Views mini pager.
Comment #15
Jorrit CreditAttribution: Jorrit commentedIs there are reason why the patch can't be applied to 7.x before I or someone else ports it to 8.x?
Comment #16
xjmWell, per the core backport policy, issues should be fixed first in D8 and then backported. Since Views in D7 is in contrib, though, I guess it's up to @dawehner to decide whether #11 can go into 7.x-3.x as-is. However, this issue should stay in the core queue for a complete D8 fix since it's apparently a blocker for an accessibility bug.
Comment #17
dawehnerI think it's okay to first get the patch in 7.x in.
Comment #18
dawehnerCommitted that patch to 7.x-3.x and working on a 8.x patch.
Comment #19
dawehnerSadly the mini pager is horrible broken at the moment (it doesn't work for multiple reasons).
The menu_rebuild is something which needs to be done, for whatever reason at the moment.
Comment #20
aspilicious CreditAttribution: aspilicious commentedI can't see a pager? Why is this thing green? What am I doing wrong?
Comment #21
dawehnerI can't reproduce that, how does this happen?
Comment #22
aspilicious CreditAttribution: aspilicious commentedI create a new view, set the number of items to 2 on the mini pager. Thats it.
Comment #23
dawehnerI have no idea how/why this happens, i can't reproduce that at all.
Comment #24
aspilicious CreditAttribution: aspilicious commentedI failed to apply the patch correctly. After a 2nd try I managed to get it working. Party!
All looking good now.
Comment #25
tim.plunkettThis hunk is wrong :(
Comment #26
dawehneroh, corrected that.
Comment #27
aspilicious CreditAttribution: aspilicious commentedCan't use @see like this should be "See"
Still incorrect :p
Comment #28
dawehnerHe, now it looks less ugly :)
Comment #29
aspilicious CreditAttribution: aspilicious commentedOw I hate to do this, the brackets needs spaces in front of them
Comment #30
dawehnerYeah they are looking awkward.
Comment #31
aspilicious CreditAttribution: aspilicious commentedUnless I missed something this is rdy to go!
Comment #32
catchIt's a little bit odd that Mini extends Full only to delete stuff. How hard would it be to make Full extend Mini and add stuff?
Comment #33
dawehnerWhat about making a common SqlPagerBase base class, from which both implementations extend? Assign the issue to myself.
Comment #34
dawehnerSo Full and Mini extends SqlBase now, an interdiff sadly doesn't make sense here,.
Comment #35
dawehnerUpdate status.
Comment #36
dawehner#34: drupal-1625248-34.patch queued for re-testing.
Comment #37
dawehnerFunny the changes for theme('item_list') didn't got applied to the views code.
Also the reroll was needed for things like fieldset => details.
Comment #39
dawehnerNo idea, but for some odd reason this patch doesn't work with patch -p1 but fine with git apply,
so here a new patch which can be applied as there is no rename in there.
Comment #41
dawehnerForgot to pull recently, sorry.
Comment #42
dawehner#41: drupal-1625248-41.patch queued for re-testing.
Comment #44
dawehner#41: drupal-1625248-41.patch queued for re-testing.
Comment #45
dawehner#41: drupal-1625248-41.patch queued for re-testing.
Comment #46
dawehner#41: drupal-1625248-41.patch queued for re-testing.
Comment #48
dawehnerOkay rerolled against some issues like the one which provided better naming for all this different fields.
Comment #49
dawehnerComment #50
damiankloip CreditAttribution: damiankloip commentedNice!!! This is some of the best clean up and refactoring I've seen in VDC! :)
SOrry, just the one pick. Aside from that, this looks great.
The indentation got messed up a bit here?
Comment #51
dawehnerHere is the context of the code:
I think this looks fine :)
Comment #52
damiankloip CreditAttribution: damiankloip commentedAhh, yeah, sorry :(
Comment #53
tim.plunkett#48: drupal-1625248-48.patch queued for re-testing.
Comment #55
dawehnerHopefully we will not need that long to fix bugs like that once core is stable.
Comment #56
dawehnerback to rtbc.
Comment #57
catch:)
Committed/pushed to 8.x.
Comment #58
damiankloip CreditAttribution: damiankloip commentedYeah! awesome patch Daniel.
Comment #59
dgatom CreditAttribution: dgatom commentedThanks a lot =]
Comment #60
dawehnerWow thanks, this is really really good that we got that in!
Comment #62
j0rd CreditAttribution: j0rd commentedSo just upgraded to views 3.6 on D7 and I believe the patch which was applied to D7 has a regression.
I get these errors now.
It appears this code removed quantity from theme_views_mini_pager().
There's a couple places in the code I can see this getting done
Is there any reason to not pass quantity to the theme function? I understand you don't need it for the theme implementation you're using....but someone else might decide (like me) their mini pager implementation is not simply Forward and Backwards.