feat: Pagination component #690

suzubara · 2022-06-28T22:02:11Z

SC-523

Do not be fooled by the charming avatar. This PR now belongs to @abbyoung 💀

To Do

finish updating unit tests
fix linting errors that came outta nowhere (these might resolve once jacob's carousel PR is merged and i rebase)
cypress, y u flake?

Proposed changes

Adds Pagination component based on USWDS design/rules
Adds this component to the ArticleList page
Implements pagination on Orbit Blog page (which uses ArticleList page)

Reviewer notes

You can view the pagination component on a page by visiting /about-us/orbit-blog, but what displays will depend on how many published ORBITBlog articles are in the db.

I've added some test articles to the dev environment so you should see more than one page. If you want to see more, you'll need to add and publish more articles in the cms.

You can also view the component on Storybook and use the sandbox to test how it will behave visually with different inputs.

Setup

Locally:
In ussf-portal-client, yarn services:up && yarn dev
In ussf-portal-cms, yarn dev

You'll need to populate db with articles in ORBITBlog category to see paginated results on /about-us/orbit-blog

To view Storybook, run yarn storybook in ussf-portal-client and view the Pagination components.

Code review steps

As the original developer, I have

As code reviewer(s), I have

As a designer reviewer, I have

As a test user, I have

Screenshots

shortcut-integration · 2022-06-28T22:02:14Z

This pull request has been linked to Shortcut Story #523: Add pagination to article list page.

package.json

…f-portal-client into sc-523/article-pagination

…petitive code in the future

package.json

abbyoung · 2022-07-13T23:53:08Z

Dockerfile

@@ -3,13 +3,13 @@
 FROM node:14.19.3-slim AS builder

 RUN apt-get update \
-  && apt-get -y --no-install-recommends install openssl libc6
+  && apt-get -y --no-install-recommends install openssl libc6 git ca-certificates


@esacteksab now that we have a released version of react uswds, is it ok to keep the changes you made? or do we need to revert?

revert please. I'm happy to do this here or on another branch. Which would you like?

here is best since they haven't been merged in yet!

I believe it's done.

gidjin

Changes look good. I left some minor comments, nothing blocking. I haven't done the a11y review yet but wanted the comments shared now in case I don't finish the a11y checks before you look at the PR again.

gidjin · 2022-07-14T00:06:21Z

Dockerfile


 WORKDIR /app

 COPY . .

-RUN yarn install --frozen-lockfile
+RUN rm yarn.lock && yarn install --frozen-lockfile --network-concurrency 1


suggestion: I'll defer to Barry here, but I think we can revert these now that react-uswds is released. Especially since this line removes the yarn.lock file that we have checked in which isn't ideal, though should have little effect since our package.json is locked down.

src/__tests__/pages/about-us/orbit-blog.test.tsx

gidjin · 2022-07-14T00:18:05Z

src/components/ArticleList/ArticleList.tsx

+      {pagination && (
+        <Pagination
+          pathname={window.location.pathname}
+          totalPages={pagination.totalPages}
+          currentPage={pagination.currentPage}
+        />
+      )}


nit-pick(non-blocking): Can you add a test to ArticleList.test.tsx to verify that if pagination props are passed in that the <Pagination /> component is included and has the expected props?

If it's too much trouble then feel free to skip.

src/pages/login/index.tsx

gidjin · 2022-07-14T00:35:52Z

src/components/Pagination/Pagination.tsx

+}: {
+  pathname: string
+  page: number
+


nit-pick(non-blocking): Can you remove this empty line?

src/components/Pagination/Pagination.tsx

gidjin · 2022-07-14T00:52:45Z

src/pages/about-us/orbit-blog.tsx

+  // If no query or an invalid param, return page 1
+  const currentPage = parseInt(context.query.page) || 1
+
+  const articlesPerPage = 10


Question: I wonder if this should be a param in the future? So that we can adjust the items per page. Nothing to do in this PR just future thoughts.

gidjin

LGTM.

question: I don't see an e2e test. Will you be adding one? Though that maybe simpler once we have the data seed stuff figured out. I'll approve since this isn't blocking for me but let me know if you add an e2e test that you want me to review.

abbyoung · 2022-07-14T19:05:43Z

Dockerfile

@@ -27,9 +27,6 @@ RUN yarn install --production --ignore-scripts --prefer-offline
 # E2E image for running tests (same as prod but without certs)
 FROM node:14.19.3-slim AS e2e

-RUN apt-get update \
-  && apt-get -y --no-install-recommends install openssl libc6


@esacteksab is this expected to have this PR remove all of this? or am i missing something? just don't want to delete anything important..

That was definitely a miss for me. I put it back. Thank you for catching this.

abbyoung · 2022-07-14T21:53:58Z

LGTM.

question: I don't see an e2e test. Will you be adding one? Though that maybe simpler once we have the data seed stuff figured out. I'll approve since this isn't blocking for me but let me know if you add an e2e test that you want me to review.

@gidjin I went ahead and added an e2e test for the ORBIT Blog page that is essentially a placeholder until we get some CMS data. But feels better to have one in place with a TODO note, even if incomplete.

gidjin

LGTM!

gidjin · 2022-07-15T06:27:05Z

e2e/cypress/integration/orbit-blog.spec.js

+    // TODO: Once we have data in CMS test db, we can test
+    // that articles and pagination are working as expected.


Praise: good idea on having a place holder with a TODO, should make coming back later to add the test easier to find.

jbecker01

Looks good!

…tion

## [4.5.0](4.4.0...4.5.0) (2022-07-29) ### Features * Add link to ORBIT Blog on portal ([#719](#719)) ([7ce6af9](7ce6af9)) * Create internal news page and update existing external news page ([#711](#711)) ([feadcae](feadcae)) * Edit display name ([#706](#706)) ([372ad2a](372ad2a)) * Pagination component ([#690](#690)) ([bc20cd5](bc20cd5))

Suzanne Rozier added 4 commits June 27, 2022 16:27

Started Pagination component

5cb1404

Add pagination logic

ec49e8c

Add sandbox story, tests

6b0b324

Clean up and add comments

6ab986b

suzubara requested review from abbyoung, esacteksab and gidjin as code owners June 28, 2022 22:02

suzubara commented Jun 28, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

abbyoung added 3 commits July 5, 2022 11:03

Merge branch 'sc-523/article-pagination' of github.com:USSF-ORBIT/uss…

2d4ae2a

…f-portal-client into sc-523/article-pagination

working pagination

b7e5f3b

wip need to fix stories

bb9ab00

abbyoung self-assigned this Jul 11, 2022

abbyoung added 5 commits July 11, 2022 16:37

update pagination story for new params

1c6e9b5

simplify code

cec92b5

Merge branch 'main' into sc-523/article-pagination

facd7c2

move pagination component to article list component to cut down on re…

b9b2dc1

…petitive code in the future

fix unit tests

069dbc4

suzubara requested a review from noahfirth as a code owner July 12, 2022 20:43

update test for orbit page

de494e7

abbyoung reviewed Jul 12, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

clean up test

9960f30

abbyoung changed the title ~~[WIP] Pagination component~~ feat: Pagination component Jul 12, 2022

abbyoung and others added 7 commits July 12, 2022 14:33

Merge branch 'main' into sc-523/article-pagination

e478885

rebuilding yarn.lock because of deploy weirdness

7328551

chore(deps): merge conflict and stuff

d62b2a6

chore: need this

4d37d9c

chore: appease yarn because we did bad things with package.json

ef432b9

chore: forgot this lovely thing

ea8126f

yarn.lock needed updating

2a50fbd

abbyoung added 2 commits July 13, 2022 15:00

merge main

9effc74

fixing type and lint error for headinglevel in accordion

f7c12b4

abbyoung reviewed Jul 13, 2022

View reviewed changes

gidjin reviewed Jul 14, 2022

View reviewed changes

fix heading level for accordion i mistakingly changed

3b1b5a6

gidjin previously approved these changes Jul 14, 2022

View reviewed changes

esacteksab added 2 commits July 14, 2022 10:58

Merge branch 'main' into sc-523/article-pagination

5f8eb6c

chore: reverting Dockerfile stuff

6a12ea3

esacteksab dismissed gidjin’s stale review via 6a12ea3 July 14, 2022 15:59

abbyoung reviewed Jul 14, 2022

View reviewed changes

esacteksab and others added 4 commits July 14, 2022 14:24

chore: putting this back, ooops

27ed5fa

cleaning up style

27f2e49

add test for validating pagination in article list

4b957cf

add initial test/placeholder for orbit blog e2e

8705c72

abbyoung requested a review from gidjin July 14, 2022 22:14

gidjin approved these changes Jul 15, 2022

View reviewed changes

abbyoung requested review from jbecker01 and jcbcapps July 15, 2022 16:20

jcbcapps approved these changes Jul 15, 2022

View reviewed changes

jbecker01 approved these changes Jul 19, 2022

View reviewed changes

jcbcapps and others added 3 commits July 19, 2022 08:56

Merge branch 'main' into sc-523/article-pagination

c17ac5d

Merge branch 'main' into sc-523/article-pagination

62d70f6

Merge remote-tracking branch 'origin/main' into sc-523/article-pagina…

a29cb5a

…tion

gidjin merged commit bc20cd5 into main Jul 25, 2022

gidjin deleted the sc-523/article-pagination branch July 25, 2022 20:59

gidjin mentioned this pull request Jul 29, 2022

chore(release): 4.5.0 #726

Merged

gidjin mentioned this pull request Sep 15, 2022

feat: Add new bounded pagination component trussworks/react-uswds#1920

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Pagination component #690

feat: Pagination component #690

suzubara commented Jun 28, 2022 •

edited by jbecker01

Loading

shortcut-integration bot commented Jun 28, 2022

abbyoung Jul 13, 2022

esacteksab Jul 14, 2022

abbyoung Jul 14, 2022

esacteksab Jul 14, 2022

gidjin left a comment

gidjin Jul 14, 2022

esacteksab Jul 14, 2022

gidjin Jul 14, 2022

gidjin Jul 14, 2022

gidjin Jul 14, 2022

gidjin left a comment

abbyoung Jul 14, 2022

esacteksab Jul 14, 2022

abbyoung commented Jul 14, 2022

gidjin left a comment

gidjin Jul 15, 2022

jbecker01 left a comment

		// TODO: Once we have data in CMS test db, we can test
		// that articles and pagination are working as expected.

feat: Pagination component #690

feat: Pagination component #690

Conversation

suzubara commented Jun 28, 2022 • edited by jbecker01 Loading

SC-523

Do not be fooled by the charming avatar. This PR now belongs to @abbyoung 💀

To Do

Proposed changes

Reviewer notes

Setup

Code review steps

As the original developer, I have

As code reviewer(s), I have

As a designer reviewer, I have

As a test user, I have

Screenshots

shortcut-integration bot commented Jun 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gidjin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gidjin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abbyoung commented Jul 14, 2022

gidjin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbecker01 left a comment

Choose a reason for hiding this comment

suzubara commented Jun 28, 2022 •

edited by jbecker01

Loading