Add pagination to publication page #2299
Conversation
Update the `FilteredEditionsPresenter` class to support paginating the result returned by the `#editions` method. If no page is specified then the first page of results will be returned. Update the tests to make it clearer when reading when the result has been converted to an array.
Add backend support for paginating the results used for the publications page. This consists of a "page" parameter that can be submitted to the controller, which will pass the value through to the `FilteredEditionsPresenter`. Adding pagination controls to the UI will follow in a subsequent commit.
Add a hard-coded sort order to the editions returned by the `FilteredEditionsPresenter`, from most-recently updated->least recently updated (this is the same as the default sort order of the legacy page that is being replaced). Does not provide a means of changing the sort order for now. Update a test that isn't testing the order of results to be order-agnostic.
mtaylorgds
force-pushed
the
publications-page/pagination
branch
from
573b26d to
4d98e3b
Compare
mtaylorgds
force-pushed
the
publications-page/pagination
branch
6 times, most recently
from
22a3036 to
af83b7d
Compare
| .govuk-pagination__item.govuk-pagination__item--current { | ||
| color: #ffffff; | ||
| } | ||
| } |
There was a problem hiding this comment.
We should only really need the .govuk-pagination__item--current class name here: given the naming system it doesn't need to be nested in or extending any other class.
Also we should use the govuk-colour("white") variable to set the colour.
There was a problem hiding this comment.
Think I'd literally just use
.govuk-pagination__item--current {
color: govuk-colour("white");
}
Should be specific enough since --current is extending govuk-pagination__item
There was a problem hiding this comment.
Good point, I've made that change now.
|
I've just made one small comment in the CSS. Otherwise it all looks good. I'm still a bit concerned about implementing a pagination that is slightly different to Whitehall in behaviour and appearance and feel we really ought to be extending that component for use here. But maybe that's something for the future if necessary. |
mtaylorgds
force-pushed
the
publications-page/pagination
branch
from
af83b7d to
a828ede
Compare
I've pushed the change you suggested.
It's not a component per se in Whitehall, it still uses the same external library (Kaminari) for pagination, it just uses a helper to generate a "pagination_hash". I don't really understand the Whitehall code, which is partially why I'm reticent to reuse it here—I don't understand what value it provides over what I've done here, which is simple styling changes. |
Yes, I was just looking at it as well. Given there isn't a design for pagination I agree with your point here. 👍 |
mtaylorgds
force-pushed
the
publications-page/pagination
branch
3 times, most recently
from
83ee341 to
4dc2694
Compare
Add (unstyled) pagination controls to the publications page. Styling to follow. Use capybara minitest assertions for improved failure readability when asserting on page content (vs the plain old minitest assertions used in other integration tests).
mtaylorgds
force-pushed
the
publications-page/pagination
branch
from
4dc2694 to
879e274
Compare
|
@davidtrussler Have updated some integration tests to turn the publications page toggle off if they navigate to that page, as some of the tests were failing depending on what order the tests were being run in. |
|
I wonder if we should check this from a Design POV before merging. I realise it's not styled exactly as Whitehall but it feels a little less elegant than that pagination on smaller viewports. It's still perfectly functional though so could be a future iteration if required.
|
| per_page: number of items to fetch per page | ||
| remote: data-remote | ||
| -%> | ||
| <span class="govuk-pagination__prev"> |
There was a problem hiding this comment.
I think using <span> elements for each of the items here is slightly problematic. If possible these should be <li> elements.
Also at smaller sizes the govuk-pagination__prev and govuk-pagination__next classes add some slightly awkward padding when used for the first and last items as well.
There was a problem hiding this comment.
I've copied the implementation from Whitehall, so this now works identically to that and uses li elements.
mtaylorgds
force-pushed
the
publications-page/pagination
branch
2 times, most recently
from
9fbf365 to
c748368
Compare
Copy the way Whitehall Publisher overrides Kaminari's partials to style its pagination components. It would be good for the govuk_publishing_components gem to have a pagination control which both apps could use, but implementing that would take some time so we will create a new ticket for this.
mtaylorgds
force-pushed
the
publications-page/pagination
branch
from
c748368 to
827341b
Compare
Add pagination controls underneath the publications table on the root path of Mainstream Publisher.