Pagination/Pager style update

Needs final design specs

I will work on this 🤠

Seven screenshot:

Test module produces this markup is here.

Still missing styles for the mini pager. There's also a problem with the sizing of the page numbers since the system fonts aren't necessarily monospaced.

1. +++ b/css/src/components/pager.css
   @@ -2,44 +2,63 @@
   -  margin: 0.25em 0 0.25em 1.5em; /* LTR */
   

   We'll inherit margin: 0.25em 0 0.25em 1.5em; from elements.css. I don't think that we'll need the big left/right margin

2. +++ b/css/src/components/pager.css
   @@ -2,44 +2,63 @@
   +  margin: 1rem;
   

   We're losing space with this approach, let's change this to margin-top: 1rem, margin-bottom: 1rem;

3. +++ b/css/src/components/pager.css
   @@ -2,44 +2,63 @@
   +  margin: 0 0.5rem 0 0;
   

   I'd prefer margin-right: 0.25rem margin-left 0.25rem here, that's symmetrical.

4. +++ b/css/src/components/pager.css
   @@ -2,44 +2,63 @@
      margin: 0;
   

   Just margin-right: 0.

5. +++ b/css/src/components/pager.css
   @@ -2,44 +2,63 @@
   +  border-radius: 38px;
   

   where is this dimension coming from?

6. +++ b/css/src/components/pager.css
   @@ -2,44 +2,63 @@
   +  padding: 0.30rem 0.75rem;
   

   If you add only a minimal right and left padding for page number links, most of your issues with these will gone

7. +++ b/css/src/components/pager.css
   @@ -2,44 +2,63 @@
   +  min-width: 32px;
   

   Space var? rem?

8. +++ b/css/src/components/pager.css
   @@ -2,44 +2,63 @@
   +  box-shadow: 0 0 0 3px #26A769;
   

   Pls use a variable!

9. +++ b/css/src/components/pager.css
   @@ -2,44 +2,63 @@
   +  background: #E6ECF8;
   

   Variable!

10. +++ b/css/src/components/pager.css
    @@ -2,44 +2,63 @@
    +  border: 2px solid #fff;
    

    Variables!

11. +++ b/css/src/components/pager.css
    @@ -2,44 +2,63 @@
    +  margin: -2px -2px;
    

    I don't see why is this negative margin needed. Shorthand.

12. +++ b/css/src/components/pager.css
    @@ -2,44 +2,63 @@
    +  border-radius: 2px;
    

    Variable?

13. +++ b/css/src/components/pager.css
    @@ -2,44 +2,63 @@
    +  background: #0048DC;
    +  color: #fff;
    

    Variables here!

14. +++ b/css/src/components/pager.css
    @@ -2,44 +2,63 @@
    +  margin: 0 0.5rem 0 0;
    ...
    +  margin: 0 0 0 0.5rem;
    

    Dont overqualify these, just use margin-right and margin-left.

15. +++ b/templates/pager.html.twig
    @@ -0,0 +1,119 @@
    +        <li class="pager__item pager__item--first">
    ...
    +        <li class="pager__item pager__item--previous">
    ...
    +        <li class="pager__item pager__item--next">
    ...
    +        <li class="pager__item pager__item--first">
    

    If you add a common modifier for these, you will be able to manage the padding of these items separated from page number links

16. The two-row design of narrow screen sizes is completely missing. Suggestion: wrap first prev into a .pager__back element (same for next and last links with a .pager__forward), and you'll be able to solve it by flex.
    • Add
      

          display: flex;
          flex-wrap: wrap;
          justify-content: center;

      to .pager__items

    • add
      

          order: 1;
          width: 50%;
          justify-content: flex-end; // flex-start for .pager__forward
          display: flex;

      to the new .pager__back

Publishing the diff asked by @lauriii on slack.

I cannot see the advantages of adding SVGs to the template level. We don't have it anywhere else (inconsistency) and I'm sure that it's a bad practice for icons (but cannot remember the arguments).

Next iteration. This makes the pager links 10 and above follow the style guide.

Re #13:

1. RTL pagers aren't aligned exactly to center
2. RTL arrow icons needs work (I'm sure that we shouldn't add icons to the template and this is a good example why not to do that)
3. first/prev/next/last pager links have margin issues on lower screen sizes

Screenshots attached.

Just a re-roll so it applies again for the moment.

And here it is with the right to left issues fixed, or at least, taking into account right to left equally as right to left.

Forgot a language-direction-specific section of the pagers.

And with this patch, whitespace-only text nodes are avoided by using Twig's {% spaceless %}. This may help address huzooka's 3, pager link margin issues on smaller screen sizes, but i'm not sure.

This looks good to me now (and it's the last i'll have a chance to work on it for a while, so i hope it's useful). Thanks all for the work on Claro!

#18 patch looks good for me. Just 1 notice: look at my screen, on mobile resolution focus styles of one element are overlapping another, looks like not enough margin between elements ?

1. +++ b/css/src/components/pager.css
   @@ -2,44 +2,107 @@
   +  padding-right: 9px;
   +  padding-left: 9px;
   

   Maybe we should use var(-space-xs) which is 8px instead

2. +++ b/css/src/components/pager.css
   @@ -2,44 +2,107 @@
   +  outline: 1px dotted transparent;
   

   = outline: unset maybe ?

Re #18:

This is still wrong, issues highlighted in #14 aren't addressed yet.

1. RTL pagers aren't aligned exactly to center
2. RTL arrow icons are still wrong
3. first/prev/next/last pager links have margin issues on lower screen sizes
4. Canvas of the SVG icons isn't sized properly.

Screenshots attached.

P.s: Please, do not add these icons by the template!

What was done:

1. first, previous, next, last icons were recreated with Adobe Ii, because previous icons had some issues with alignment (never use "stroke" attributes in svg icons !!!!)
2. removed inline svg from template and recreated with css
3. improved rtl support.
4. cleanup.

It seems like #14 has been solved by #23.

I did find another problem. Our default views configurations adds extra arrows in the link text configuration:

Maybe we could use a Twig filter to remove the arrows in the template from the text?

Removed the arrows with a twig filter in pager.html.twig. Patch + interdiff attached.

+++ b/css/src/components/pager.css
@@ -2,44 +2,143 @@
+/**
+ * Vars.
+ */
+:root {
+  --pager-size: 2rem; /* --space-m × 2 */
+  --pager-border-width: 0.125rem; /* 2px */
+  --pager-fg: var(--color-davysgrey);
+  --pager-bg: var(--color-white);
+  --pager-bg--hover: var(--color-bgblue-active);
+  --pager-bg--focus: var(--color-focus);
+  --pager-fg--active: var(--color-white);
+  --pager-bg--active: var(--color-absolutezero);
+  --pager-action-border-radius: var(--pager-border-width);
+}

Shouldn't this be moved to the `variable.css` file?

Added a patch which moved the variables of pager from pager.css to variables.css

1. --color-focus should be --color-green and moved to Base variables section
2. found problem with margin of all pager. what gives problem on mobile

i have suggestion to override margins-shadows policy. to cover design

and remove them from list items.

mini pager has own template

also fixed ellipsis in case of "Number of pager links visible" option setted to small number.

We should manually test all different views core provides in Standard installation since this is affected by configuration. We should also provide screenshots for the mini pager. We should also followup issue for updating the mini pager after the styleguide has been updated.

@lauriii identified the following core modules that are not enabled by default but which contain views that should also be tested:

• Aggregator
• Content Moderation (requires Workflows)
• Media
• Media library

Migration groups also can show a pager, but it looks like that page isn't technically a view? I'd think we should include that as well, however.

I confirmed manually that the pager configuration is the same in all views in core. Pager configuration is consistent across views in core, so testing on the content page is enough.

The mini pager looks fine, but we could probably do better so I opened a follow-up to make improvements to that.

I also rerolled the patch.

Everything spotted is using the /pager path via the helper module mentioned earlier in this issue

1. When the current item is hovered over, there are no visual changes other than the pointer changing. I suspect the intent is to have it match the hover behavior of not-current items, but .pager__link.is-active takes precedence over .pager__link.is-active
2. The alignment of the first/last/prev/next SVGs does not match the comps in Figma. While this observation might seem pixel-pedantic, I think it would impact first impressions enough to be worth doing.
   

   

3. In IE11 high-contrast mode, current items have no visual distinction and hovered-over items are only differentiated by the mouse pointer changing.
4. In IE11, the First/Prev/Next/Last links all appear after the page numbers instead of the expected First/Prev appearing on the left.
   
5. Reviewed RTL by installing Hebrew. Everything seems fine and matches the behavior of RTL pagers on Bartik, but to be on the safe side (I don't work with RTL often) I'm providing screenshots in different use cases.
   

Will return to this later in the week to review the code itself, but wanted to provide a little bit of time to potentially address the issues I found during cross browser testing.

This addresses the items I found in #39
#39.1 The previously-not-happening hover behavior with the current item now matches hover behavior of any pager items. There's currently nothing in Figma for this state, but can be updated fairly easily to accommodate any design updates. Since it's still a link (even if it goes to the same page), it should get the same visual treatment as its siblings.

#39.2 Moved the icon positions to match Figma

#39.3 Added two high-contrast-mode specific styles. Hovering over links in the pager underlines them, since the background-color effect is not visible. The current item is circled as seen in the screenshot (the focus ring in high contrast mode is a dashed square, so this won't overlap with this)

#39.4 Fixed this by adding display:block; to the row separator in .pager__items::before

Also ran into a tab order issue similar to the one reported in #3053457: Flex order breaks tabbing order on tabs . On narrow viewports, the First/Previous/Next/Last items are positioned below the page numbers, but the tab order continues to be First > Previous > page numbers > Next > Last, so tabbing away from Previous can be confusing because it jumps up a line to go to the page numbers. It does not compromise tab navigation as much as the scenario described in 3053457, the items can still be accessed. Not sure at the moment if it merits a separate followup issue or perhaps this is acceptable behavior?

The difference between tabbing order and visual order could create confusion with users with low vision users. I think it would be a good idea to open a follow-up for that.

+++ b/templates/pager.html.twig
@@ -0,0 +1,123 @@
+              {{- items.last.text|default('Last'|t)|replace({'»': ''}) -}}

We should open a follow-up in the core queue to figure out what to do for this.

It seems like the arrow isn't positioned correctly on RTL.

#42 Opened #3053752: Flex order breaks pager tab order for the tab order issue, and #3053707: Pager arrows should be easier to override in themes for the views pager arrows issue. Added a @todo to that issue in pager.html.twig

#43 The RTL arrow alignment is addressed

@lauriii pointed out that the relative positioning solution for aligning the arrows may not be ideal. To truly vertically center them, I think an approach like the details triangle alignment in #3038784: Details style update will be needed, where the SVG is the background in an absolutely positioned :before selector. This would require many more css rules, but the arrows would be truly vertically centered and would not require pushing pixels to accommodate RTL and LTR. Wouldn't be able to get to this for a bit anyway, so commenting here to see if anyone else has thoughts.

I followed @bnjmnm suggestion to make the svg a background-image instead of content. I gave the icons an arbitrary height of 1rem which was slightly bigger than the svg, and centered the background.

Flexbox seemed like a good solution for centering everything vertically.

The other issue with RTL I think was rotation, the icons were not perfectly symmetrical in that direction but scaleX looks a little better, I attached a screenshot for reference.

Background images are not visible in high contrast mode but I think it might be fine in this case because the icons act just as complementary information.

I updated the icons to the most recent versions from Figma.

I'm reviewing this

When I just tried to get the Figma it turned out that the mobile design changed and it does not show action link labels (first, previous, next, last) anymore — we aggreed on that we should drop the current flex-order tricks and focus on the wide design (it's really close).

huzooka [3:31 PM]
mobile does not show action labels anymore
(first-prev-next-last)

lauriii [3:32 PM]
I think it has been changed since we first implemented the designs

huzooka [3:34 PM]
I'm sure that until March it did

lauriii [3:34 PM]
But I don’t think we should implement the simple version at least yet
it could have it’s own problems because of views configurations
I think it should be done in a follow-up

huzooka [3:35 PM]
than we should drop the flex-order thing
that only adds problems

+++ b/css/src/components/pager.css
@@ -2,44 +2,144 @@
+.pager__item:first-child {
+  margin-left: 0; /* LTR */
+}
+[dir="rtl"] .pager__item:first-child {
+  margin-left: calc(var(--space-xs) / 2);
+  margin-right: 0;
+}
+.pager__item:last-child {
+  margin-right: 0; /* LTR */
+}
+[dir="rtl"] .pager__item:last-child {
+  margin-right: calc(var(--space-xs) / 2);
+  margin-left: 0;
+}

I'd remove these rules. This assumes that all of the items are rendered in the same row, but that is not true for narrow browser viewport sizes (or when there are a lot of page numbers displayed)

Per #50, the order: css is removed. as well as the left/right margins for .pager__item

Also removed this rule

[dir="rtl"] .pager__items {
  margin-right: 0;
}

Since the r/l margins for .pager__items are both 0 already.

+++ b/css/src/components/pager.css
@@ -99,7 +72,6 @@
-  height: 1rem;

I assume this was removed accidentally.

Visual remove in progress.

Edge high contrast: active pager item has a border issue:

This should address #57.

I tested with the patch in #58 and then changed back height to line-height to try and see this bug in high contrast mode, I didn't see the "gaps" but it is more circular now.

I liked it better with the flex order putting the next/prev at the end, but I understand why it changed with the need to wrap to multiple lines. Removing the extra margin makes sense.

If the high contrast issue was the last part I think this is RTBC unless @huzooka is still seeing a problem.

Re #58:

I had to put back line-height for Edge because it still had the broken border in high contrast mode:

Looks good! I think this is ready.

Thank you everyone! 🎉