Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guidance for pagination component #1972

Merged
merged 2 commits into from
Jun 27, 2022
Merged

Guidance for pagination component #1972

merged 2 commits into from
Jun 27, 2022

Conversation

calvin-lau-sig7
Copy link
Contributor

@calvin-lau-sig7 calvin-lau-sig7 commented Nov 4, 2021

Refine guidance for pagination component, based on design decisions and previous working group review.

Content improvements are mainly to:

  • better explain the rules 'for large numbers of pages'
  • structure the page to help users navigate the different patterns

Preview of page, with examples:
https://add-pagination--govuk-design-system-preview.netlify.app/components/pagination/

Other links:

Epic: Publish pagination contribution govuk-design-system#1985
Design and code is in this PR: Add pagination component #2610

@calvin-lau-sig7
Copy link
Contributor Author

Hi @timpaul, all. I've made some comments and suggested some edits and to the guidance. Hope I've committed them properly into this branch — thanks!

@calvin-lau-sig7 calvin-lau-sig7 changed the title Content suggestions for pagination component Guidance for pagination component Nov 5, 2021
@calvin-lau-sig7 calvin-lau-sig7 self-assigned this Nov 5, 2021
@netlify
Copy link

netlify bot commented Nov 5, 2021

You can preview this change here:

Name Link
🔨 Latest commit ce9f50a
🔍 Latest deploy log https://app.netlify.com/sites/govuk-design-system-preview/deploys/62b9a30391b9a400081475ec
😎 Deploy Preview https://deploy-preview-1972--govuk-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@calvin-lau-sig7 calvin-lau-sig7 changed the title Guidance for pagination component Publish pagination component/pattern Nov 23, 2021
@calvin-lau-sig7 calvin-lau-sig7 changed the title Publish pagination component/pattern Publish pagination contribution Nov 23, 2021
@calvin-lau-sig7 calvin-lau-sig7 changed the title Publish pagination contribution Guidance for pagination component Nov 23, 2021
@calvin-lau-sig7 calvin-lau-sig7 linked an issue Nov 23, 2021 that may be closed by this pull request
27 tasks
@calvin-lau-sig7

This comment was marked as outdated.

@frankieroberto
Copy link
Contributor

@calvin-lau-sig7

Looked into a better heading structure for the examples. What do we think of something that looks like:
[...]

I like where this is heading, but I found the distinction between the 3 different top-level headings hard to understand, with "set of pages" and "sequence of pages" initially sounding like the same thing to me?

I wonder if we could simplify to just 2 different headings describing the 2 ways of using the component, one for "content pages" (needs a better description) which would use just previous/next, and one for "results pages" (previous/next plus numbered links and optional results line)?

I can't think of a reasonable use-case for numbered links with content-y type pages?

Any thoughts @timpaul?

@simonwhatley
Copy link
Contributor

simonwhatley commented Dec 1, 2021

@timpaul in the Nunjuck examples the keys are all quoted, whereas elsewhere in the design system they're not.

For example:

{{ govukPagination({
  "previous": {
    "href": "#",
    "text": "Previous team"
  },
  "next": {
    "href": "#",
    "text": "Next team"
  },
  "items": [
    {
      "text": "1",
      "href": "#"
    },
    {
      "text": "2",
      "current": true
    },
    {
      "text": "3",
      "href": "#"
    }
  ]
}) }}

should be this:

{{ govukPagination({
  previous: {
    href: "#",
    text: "Previous team"
  },
  next: {
    href: "#",
    text: "Next team"
  },
  items: [
    {
      text: "1",
      href: "#"
    },
    {
      text: "2",
      current: true
    },
    {
      text: "3",
      href: "#"
    }
  ]
}) }}

@owenatgov owenatgov force-pushed the add-pagination branch 2 times, most recently from 7cad55a to 6ac133f Compare April 19, 2022 10:54
@owenatgov owenatgov force-pushed the add-pagination branch 2 times, most recently from 619e556 to 6489b24 Compare May 16, 2022 12:25
@calvin-lau-sig7
Copy link
Contributor Author

Putting this on blocked for now, whilst we make final refinements to the component.

@owenatgov owenatgov force-pushed the add-pagination branch 3 times, most recently from 7854807 to f19b5d3 Compare May 25, 2022 10:59
@calvin-lau-sig7
Copy link
Contributor Author

calvin-lau-sig7 commented Jun 9, 2022

This guidance content should be ready to release now, just needs GitHub approvals.

@owenatgov owenatgov force-pushed the add-pagination branch 4 times, most recently from cf57f0a to ed4aa86 Compare June 20, 2022 14:10
Copy link
Member

@christopherthomasdesign christopherthomasdesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, just one comment

36degrees and others added 2 commits June 27, 2022 13:30
Iterate content and add headings for examples

Fix typo for heading level

Fix headings

Add guidance for pagination component

Iterate content and add headings for examples

Fix typo for heading level

Fix headings

Apply suggestions from tech writer review

Co-authored-by: EoinShaughnessy <72507742+EoinShaughnessy@users.noreply.github.com>

Update pagination guidance

Updated guidance to:

- cover issues raised by working group
- clarify use case for different types of pagination - and when to use 'continue' button and back link instead

Minor edits for dashes and contractions

Tweaking paragraph breaks

Remove section about saying "page" in labels

By default, "Previous" and "next" do not include "page" so this section is no longer needed
@36degrees
Copy link
Contributor

Rebased to remove pre-release from package.json and to tidy up the commit messages.

@domoscargin domoscargin merged commit 494152c into main Jun 27, 2022
@domoscargin domoscargin deleted the add-pagination branch June 27, 2022 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Publish pagination contribution