Convert Olivero's pager to use single directory components

Created on 7 June 2023, over 1 year ago
Updated 22 July 2024, 5 months ago

Olivero's pager component should be using SDC. Let's migrate it. The current code lives in core/themes/olivero/templates/navigation/pager.html.twig

📌 Task
Status

Needs work

Version

11.0 🔥

Component
Olivero 

Last updated about 2 hours ago

Created by

🇺🇸United States mherchel Gainesville, FL, US

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mherchel
  • Assigned to xenophyle
  • 🇺🇸United States sharkbaitdc

    Initial pass at converting the pager for sdc

  • Issue was unassigned.
  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States mherchel Gainesville, FL, US

    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.

  • Status changed to Needs review over 1 year ago
  • last update over 1 year ago
    Custom Commands Failed
  • 🇮🇳India gauravvvv Delhi, India

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

  • Status changed to Needs work over 1 year ago
  • 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.

  • 🇮🇳India _utsavsharma

    Tried to fix failures in #8.

  • last update over 1 year ago
    Build Successful
  • First commit to issue fork.
  • Merge request !8768Issue #3365391: Add Pager SDC. → (Open) created by Unnamed author
  • Pipeline finished with Failed
    5 months ago
    Total: 583s
    #224559
  • Status changed to Needs review 5 months ago
  • Status changed to RTBC 5 months ago
  • 🇮🇳India Vishal Choudhary Dharmshala

    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.

  • Status changed to Needs work 5 months ago
  • 🇬🇧United Kingdom catch

    One question on the MR.

  • Pipeline finished with Failed
    5 months ago
    Total: 540s
    #227451
  • 🇬🇧United Kingdom catch

    Looks like the library definition still needs updating too.

  • Pipeline finished with Failed
    5 months ago
    Total: 664s
    #228513
  • Status changed to Needs review 5 months ago
  • Status changed to Needs work 5 months ago
  • 🇺🇸United States smustgrave

    Appears to have multiple test failures.

  • Pipeline finished with Failed
    5 months ago
    Total: 456s
    #231960
Production build 0.71.5 2024