Convert Olivero's pager to use single directory components

Initial pass at converting the pager for sdc

Thanks for all of the work on this! Initial review below:

1. +++ b/core/themes/olivero/components/pager/pager.components.yml
   @@ -0,0 +1,21 @@
   +$schema: https://git.drupalcode.org/project/sdc/-/raw/1.x/src/metadata.schema.json
   

   Lets get rid of the schema, status, and group keys here.

2. +++ b/core/themes/olivero/components/pager/pager.components.yml
   @@ -0,0 +1,21 @@
   +slots:
   

   There's no pager block, so this is not needed

3. +++ b/core/themes/olivero/components/pager/pager.twig
   @@ -0,0 +1,129 @@
   +#}
   

   Lets keep these comments within the pager.html.twig

4. +++ b/core/themes/olivero/components/pager/pager.twig
   @@ -0,0 +1,129 @@
   +	{% if items %}
   ...
   +								{{ current == key ? 'Current page'|t : 'Page'|t }}
   

   Need to use spaces not tabs.

5. +++ b/core/themes/olivero/components/pager/pager.twig
   @@ -0,0 +1,129 @@
   +                {% include "@olivero/../images/pager-previous.svg" %}
   

   Lets 1) convert this to a twig function, and 2) bring all of the pager's images into the component and reference them using componentMetadata.

6. +++ b/core/themes/olivero/components/pager/pager.twig
   @@ -0,0 +1,129 @@
   +							</span>
   

   Need to use spaces not tabs.

7. +++ b/core/themes/olivero/templates/navigation/pager.html.twig
   @@ -30,90 +30,12 @@
   +{% embed "olivero:pager" with {
   

   Since we're not using any blocks here, let's use an include function.

Addressed feedbacks from #6, attached interdiff for same. please review

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Tried to fix failures in #8.

In this scenario, I have created two nodes and a view to show a node using a pager.
After saving the view go to the page URL and show the Output like that
Screenshot Attached:

To See this Output May be Moved to RTBC
Thanks.

One question on the MR.

Looks like the library definition still needs updating too.

Appears to have multiple test failures.

First-time contributor - Refactoring the pager component
Hi, this is my first time contributing to Drupal. 😊

This patch includes updates to the pager.component.yml and pager.twig files
If there’s anything I’ve done incorrectly or any feedback you have, please let me know. Thank you!

ignaciofarre → closed merge request !11011

ignaciofarre → changed the visibility of the branch drupal-3502353-3502353-2 to hidden.

ignaciofarre → changed the visibility of the branch 3502353-2 to hidden.

Hello and thanks for the first time contribution.

So with all issues they should use the standard issue template, the template that's there when you create a new ticket. Even if sections aren't relevant like API change leave it and put a NA under it.

Will probably need SDC maintainer eyes, will ping them.

A few feedbacks from a first quick look:

Undefined objects

This is not enough to be understood by developers using your component and leveraged by display builders ( UI Patterns 2 → , the upcoming Experience Builder...):

        previous:
          type: object
          title: Previous page
          description: Represents the previous page relative to the current page, typically with a link to the preceding page in the pager.

You need to tell what is inside the object, which properties.

Buy the way, don't feel obligated to mimic the current Twig logic and model of core/themes/olivero/templates/navigation/pager.html.twig , this is also the occasion of proposing a cleaner model and do the mapping when you include the component.

Alien classes

Are you sure layout--content-medium belongs to the component template ?

attributes.addClass(['pager', 'layout--content-medium'])

it looks foreign for me.

Maybe you can inject it from the presenter template instead:

{% include 'olivero:pager' with {
  attributes.addClass('layout--content-medium'),

Use include function instead of include tag

According to Twig documentation, it is recommended to use the include function instead as it provides the same features with a bit more flexibility: https://twig.symfony.com/doc/3.x/tags/include.html

Proper use of icons

It is a bit weird to use componentMetadata as a workaround to get icon path but I understand why you did that:

 {{ include (componentMetadata.path ~ '/images/pager-previous.svg') }}

However, there is a new Icon API in Drupal Core since Drupal 11.1: ✨ Add an icon management API Active

So, you can add an icon pack in olivero.icons.yml (naive, non tested, example):

olivero:
  label: "Olivero icons"
  extractor: svg
  config:
    sources:
      - path/to/your/icons/
  settings: {}
  template: >-
    <svg {{ attributes }}>{{ content }}</svg>

Adn then call your icon from your template:

 {{ icon ('olivero', 'pager-previous') }}

It is a new API. I would be happy to help if necessary.

Edited MR. Attempt to make code more comprehensible.

Hi Pierre,
Thank you for your review and comments—I really appreciate the feedback.
I’ve made some updates based on the suggestions:
1. I added a new component (`pager_item`) and structured the render arrays in the preprocess function. This seemed necessary because of the complexity, but I’m not entirely sure if this was the best approach. Please let me know if there’s room for improvement.
2. I also added the view mini pager template since it was previously unstyled.

Unfortunately, I wasn’t able to make the icons work. Any guidance or feedback on this would be greatly appreciated.

Lastly, thank you, Andrew, for reviewing the text as well. English isn’t my first language, and I’d really appreciate another review of the text if possible.

Please let me know if I can improve or update the code further—I am eager to learn, so don’t hesitate to share any suggestions.
Thanks again!

Hi Ignacio,

Undefined objects

I added a new component (`pager_item`) and structured the render arrays in the preprocess function. This seemed necessary because of the complexity, but I’m not entirely sure if this was the best approach. Please let me know if there’s room for improvement.

Creating a pager_item component is not a bad idea. There is plenty of situations when we want a "sub component" repeated in a parent component' slot:

• slides in a slider component
• items in an accordion component
• tab in a tabs component

However, in this case, I don't believe it is the best choice:

there is not slot in pager_item so the repeated data structure can be expressed with JSON schema
this data structure already exists in Drupal: pager links. it just need to be defined properly instead of a simple type: object

Also, if you decide to keep pager_item, the BEM HTML class name must not be an element of pager:

{% set classes = [
  'pager__item',
  'pager__item--' ~ itemType
] %}

but its own block:

{% set classes = [
  'pager-item',
  'pager-item--' ~ itemType
] %}

See: https://getbem.com/naming/

Proper use of icons

Unfortunately, I wasn’t able to make the icons work. Any guidance or feedback on this would be greatly appreciated.

This new API (available since Drupal 11.1) is easy to use once the icon pack is defined.

First, you need to have a clearer organisation of your icons. You actually have:

• images/icons/pager-first.svg (without the SVG wrapper, that's weird, why?)
• images/icons/pager-previous.svg (without the SVG wrapper, that's weird, why?)
• components/paginator/pager_item/images/pager-first.svg
• components/paginator/pager_item/images/pager-previous.svg

I have no opinion about where to keep those 2 icons, but it seems there is some duplications to fix, so I am proposing to move components/paginator/pager_item/images/*.svg to images/icons/*.svg , replacing the existing files.

Then you can create a olivero.icons.yml file at the root of the theme with:

olivero:
  label: "Olivero"
  extractor: svg
  config:
    sources:
      - images/icons/*.svg
  settings: {}
  template: >-
    <svg {{ attributes }}>{{ content }}</svg>

And try to print {{ icon ('olivero', 'pager-previous') }} in a template. Is it working?

If yes, it is a good start :) If no, let's check this together.

Linting & and checking

There is no official Twig linter for Drupal Core yet, but you can run the Twig files of the components added by you (and only those ones) with 📌 Adopt friendsoftwig/twigcs for Twig coding standards Needs review

Also, https://www.drupal.org/project/sdc_devel → can be useful to check your component (definition & template) :

• there is a report page: /admin/reports/ui-components
• and a drush command: drush sdv olivero