Views ordered list numbers restart at one when pager used

Thanks @jwilson - your template tweak for views-view-list.html.twig in 11 worked really well.

Awesome @robcarr!

I'm thinking we could reopen this and switch it to the core Drupal issue queue's Views sub-system.

It was closed years ago by Merlin of Chaos with this reasoning:

Views can't be held responsible for that. There's no really easy way to change that, I don't think, though maybe adding a style directive via theming I think CSS can tell the ordered list what number to start on.

However, the approach in #11 proves that this can (and should) be done really easily with an HTML attribute in the template, not in the theme's CSS.

For the initial question in the IS, isn't this something you would use the 'View result counter' field for? That would not get it right on a HTML level but it would show the correct numbers right?

IS:

when the ordered (numbered) list option is used and pager displays a second page the items are numbered again from 1 ... rather than continuing at the next higher number. Is there somewhere to change this behaviour?

#15:

isn't this something you would use the 'View result counter' field for?

Sure, you could use view result counter, but not if you want semantic html list. Effectively, if you were to use the ordered (numbered) list option with the view result counter field, then you end up with two sets of numbers, one from the HTML <li> itself, and the other from the field value, and as described above, from page two onward the two values would start to diverge. Using view result counter with numbered list would only require further hacking the numbered list functionality to hide the ordinals/bullets in your theme's CSS. This workaround doesn't do any thing for the viability of the numbered list feature out of the box.

Seems to have a test failure.

Also could the issue summary be updated please

I cannot make heads or tails of the error. It seems unrelated.

1) Drupal\Tests\system\Functional\Theme\ToolbarClaroOverridesTest::testClaroAssets
Failed asserting that false is true.

/var/www/html/core/modules/system/tests/src/Functional/Theme/ToolbarClaroOverridesTest.php:141

Line 141 looks like this:

    // Confirm toolbar templates are loaded from Claro.
    $this->assertTrue($this->getSession()->getPage()->find('css', '.toolbar')->hasClass('claro-toolbar'));

We haven't touched anything remotely related to Drupal toolbar in the Claro theme with the changes here.

This feels like a feature request to me, and something that should be optional. It seems strange to have a page with an ordered list that starts from an arbitrary number? I don't think it's safe to assume that everyone using ordered lists would want it to work that way, which is also supported by the lack of activity on this issue in 15 years.

I'm having trouble making it all the way to your side to re-classify this as a feature request instead of a bug. (I'd honestly be very interested to see a use case where it makes sense to start ordered, paged output back at 1 on each subsequent page load.) That being said, it is a valid point that this should probably be refactored to be optional, and on by default for new new views, but off by default for existing views, so folks can make a decision whether to opt in on a case-by-case basis.

I'd honestly be very interested to see a use case where it makes sense to start ordered, paged output back at 1 on
each subsequent page load.

I think I just consider each page distinct, but it sounds like you are considering them states of a single page.

I will concede I haven’t used the ordered list formatter very much in my 12+ years of Drupaling but just because it was not needed. I use the results summary far more often. Just anecdotal but could be it’s not widely used generally and that’s why there isn't much demand for this.

I think I just consider each page distinct, but it sounds like you are considering them states of a single page.

I haven’t used the ordered list formatter very much

Anecdotally, I've had the need for this several times over the years. So much so that I decided to revive this issue and share my work. I use ordered lists on paged search results, where yes each page in the search results is part of a larger context. This fix is also really helpful when using "Views Load More" or "Views Infinite Scroll" module, where new results are loaded into the current page via ajax without a round trip to the server.

Keep in mind, I'm implementing designs from UX teams with limited knowledge of Drupal, and this pattern of incrementing the index of give search results across pages just makes sense intuitively. It is also worth noting that this issue was closed 16 years ago by Earl, the original author of Views, which could also explain why not more people are subscribed to this issue. The fact that there were 10 comments on the issue after it was originally closed is a decent indicator that there is a need, however small, for this fix.

I rely on the method in #11 (the Twig template override) as the go-to solution on many projects because it is more semantic and properly announced and easily navigable by screen readers, as opposed to having to go with the "views result counter" approach in #15 inside each row and using the "Unformatted" Views display format, which then requires more theming effort to restore basic formatting styles that you get for free with an <ol> and <li>.

Infinite scroll is certainly a good use case :)

I guess I just haven't come across designs for numbered search. I sure wish I could say I was implementing designs from teams with knowledge of Drupal, do those exist? LOL. Just lucky I guess!

Perhaps the implementation approach should be moved out of the template and into a preprocess hook, so that all themes will benefit from this fix.

At one point Wim Leers had a number of issues which were moving logic from preprocess to templates, see [#701667] and #3260399: [meta] Theme/render system cleanup → , which also fits with the way SDCs work. So having the logic in the template might be fine.

This probably needs tests.

This is does need yes.

This was a random issue for Bug Smash today. It has been recently updated so just adding tag.

Didn't test, but what happens on a View with no pager and you try to use view.pager.current_page? Should it check for a pager before using this? And does the Some pager have view.pager.current_page?

Just looked at existing coverage that might make testing this easier, but alas, there seems to be very little coverage for the list style, just \Drupal\Tests\views\Kernel\Plugin\StyleHtmlListTest as far as I could quickly find. No tests for 'ol' list at all.

Still needs tests but tried an alternative approach to add the attribute via code vs twig. Now we don't need a CR for the twig changes.

smustgrave → changed the visibility of the branch 293295-views-ordered-list to hidden.

Goes it a shot with the tests.

This definitely looks a lot cleaner with the logic inside a preprocess function, although back in #28 it was stated that the logic might be better in twig IMO it makes sense here to be in a preprocess function.

Change looks straightforward.
Test only fails as expected.
I manually tested following the reproduction steps in the IS and it does not start at the right number after paging on 11.x but does start at the right point on this branch so this works.

For @lendude in 30, I also tested with no pager present and saw no issues so I think that is not an issue.
First glance I thought the class changes were out of scope but it's so the attributes exist even if a custom class isn't set.

RTBC

Thank everyone for pushing this forward.

First glance I thought the class changes were out of scope but it's so the attributes exist even if a custom class isn't set.

Good point. Also worth pointing out that initializing the Attributes may also have the positive knock-on effect of resolving #2845400-24: Adding attributes to views-view-list.html.twig doesn't work if Views List class Style option is empty → 🙌! But it's worth confirming and double-checking because the patch there is similar to the work done here, but has one additional hunk not present in this issue. Even if it doesn't completely solve that issue, maintainers could consider bringing over issue credit from that issue.

catch → credited daniel korte → .

Committed/pushed to 11.x, thanks!

catch → closed merge request !10966

Automatically closed - issue fixed for 2 weeks with no activity.