feat: Dynamic landing pages #1143

jcbcapps · 2023-11-03T16:21:56Z

SC-2617

Proposed changes

adds in page navigation component to Landing Page
adds [article] folder with index.tsx under src/pages/landing
adds unit tests
styles responsive layout, dark mode, and visual formatting
adds labels to the article portion of the getLandingPage query
adds styles for sticky header
adds in page navigation to storybook
updates styling of docs accordion to match figma
updates styling of background fills to match figma

Reviewer notes

E2E tests: https://github.com/USSF-ORBIT/ussf-portal/pull/332

Setup

Start the system

yarn services:up
yarn dev
cd ../ussf-portal-cms
yarn dev

Login to the portal http://localhost:3000

Start storybook

yarn storybook

Login to storybook http://localhost:6006, though the command above should open it for you

Code review steps

As the original developer, I have

Met the acceptance criteria
Created new stories in Storybook if applicable
Created/modified automated unit tests in Jest
Created/modified automated E2E tests
Followed guidelines for zero-downtime deploys, if applicable
Use ANDI to check for basic a11y issues

As a reviewer, I have

Check out our How to review a pull request document.

Screenshots

…t to dynamic LP

shortcut-integration · 2023-11-03T16:21:59Z

This pull request has been linked to Shortcut Story #2617: Add Landing Page to Portal.

…/ussf-portal-client into sc-2617/dynamic-landing-pages

jcbcapps · 2023-11-07T17:16:00Z

src/pages/landing/[landingPage]/index.tsx

+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const pageContent = (): any => {


The content prop for the InPageNavigation component expects a type of Element & string for some reason that I don't understand. I tried to remedy this a few other ways, but nothing else worked so I added this any and explicitly ignored the type.

jcbcapps · 2023-11-08T16:22:22Z

src/pages/landing/[landingPage]/index.tsx

+        {articles.length >= 1 && (
+          <ArticleList articles={articles} landingPage={true} />
+        )}


Unfortunately, I had to do a bit of prop drilling here. The landingPage prop gets passed down to ArticleListItem and is used to link to the dynamic path for an article that is related to a landing page. I did this to save a bit of time on what would probably need to be a larger refactor. Let me know if you see a better solution!

jcbcapps · 2023-11-08T16:58:27Z

src/components/PageHeader/PageHeader.tsx

-  searchQuery?: string
+  searchDisplay?: boolean


This prop wasn't being used, so I changed it to be a boolean that we can use to toggle the display of the Search component

jcbcapps · 2023-11-08T16:59:37Z

src/pages/landing/[landingPage]/[article]/index.tsx

+            { path: '/', label: 'Service portal home' },
+            { path: '/landing', label: 'Landing pages' },
+            {
+              path: `/landing/${breadcrumbNavItems[2]}`,
+              label: `${breadcrumbNavItems[2].toUpperCase()}`,
+            },
+            {
+              path: `/landing/${breadcrumbNavItems[2]}/${breadcrumbNavItems[3]}`,
+              label: `${breadcrumbNavItems[3]}`,
+              current: true,


I thought it would be fine to hardcode the array positions here for now, but we could write some logic to make it a bit more dynamic in the future if need be.

jcbcapps · 2023-11-08T17:02:37Z

src/pages/landing/[landingPage]/[article]/index.tsx

@@ -18,6 +18,7 @@ const LandingPageArticle = ({
  return (
    <>
      <PageHeader searchDisplay={false}>
+        <h1>{breadcrumbNavItems[2].toUpperCase()}</h1>


I wasn't sure about adding this h1 here, but I thought displaying what landing page the article is nested under could be beneficial.

jcbcapps · 2023-11-08T17:32:43Z

src/components/SingleArticle/SingleArticle.tsx

          {category === 'InternalNews' ? (
            <Category category={CONTENT_CATEGORIES.NEWS} />
-          ) : (
+          ) : category !== 'LandingPage' ? (
            <Tag className={`${tagStyles.Category}`}>{category}</Tag>
-          )}
+          ) : null}


When looking at an article nested under a landing page, the LandingPage category was being shown. I thought that might be confusing for the user so I added this to hide it.

shkeating · 2023-11-15T18:01:31Z

@jcbcapps to do: add page titles to all pages involved in this. We want to set it to the name of the landing page, not the default of USSF Portal

…n and dynamically resizing based on element heights using refs

…nto sc-2617/dynamic-landing-pages

…oves active styles from in page nav as it is flashing the active state in weird ways

…nto sc-2617/dynamic-landing-pages # Conflicts: # src/layout/DefaultLayout/LandingPageLayout.module.scss # src/pages/landing/[landingPage]/index.tsx

shkeating

shauna and anna approval attained in design <> eng sync today

gidjin

LGTM!

gidjin · 2023-11-17T01:36:59Z

src/pages/landing/[landingPage]/index.tsx

+  const session = await getSession(context.req, context.res)
+  const user = session?.passport?.user
+
+  if (!user) {
+    return {
+      redirect: {
+        destination: '/login',
+        permanent: false,
+      },
+    }
+  }


Question: Did we need to do a redirect here? Asking since I don't know that we do it elsewhere.

Good catch, I don't think we do. I'm currently working on adding tests on another branch and I can remove this there.

jcbcapps and others added 9 commits October 31, 2023 13:50

Add baseline page

61e61ec

Add getLandingPages query and display results

72bf3f7

Merge branch 'main' into sc-2626/landing-page-index

8fb14ef

Refactor PageNav and add LD flag

30508ee

Add baseline for dynamic landing page

b1287ce

Swap landing page index to use DefaultLayout and add LandingPageLayou…

7fcfb68

…t to dynamic LP

Remove CustomModal from layout

5469c14

Add baseline dynamic landing page

1f7456f

Style updates

64abb78

jcbcapps and others added 9 commits November 3, 2023 11:24

Merge branch 'main' into sc-2617/dynamic-landing-pages

d935603

Update styles

965be43

Add conditional rendering for each LP item

98eba7c

Merge branch 'main' into sc-2617/dynamic-landing-pages

a15df5b

Merge branch 'sc-2617/dynamic-landing-pages' of github.com:USSF-ORBIT…

e05a43a

…/ussf-portal-client into sc-2617/dynamic-landing-pages

Update page structure

ff8d8a3

Update styles for main

f956366

Remove unnecessary style

1b1fa7c

Disable type check

c41c88c

jcbcapps commented Nov 7, 2023

View reviewed changes

Add nested articles for a landing page

26c2252

jcbcapps commented Nov 8, 2023

View reviewed changes

jcbcapps added 2 commits November 8, 2023 10:24

Add GridContainer to page

6c4da41

Add breadcrumbs

9b9070b

jcbcapps commented Nov 8, 2023

View reviewed changes

Add h1 to display landing page title

80a0c57

jcbcapps commented Nov 8, 2023

View reviewed changes

Do not display LandingPage category to users

13da6a6

jcbcapps commented Nov 8, 2023

View reviewed changes

jcbcapps and others added 18 commits November 15, 2023 13:14

Update query to include status and label type

e97a047

Add page title

a059e09

adds background box

b723a50

sets up calculations to place the background box below the descriptio…

5bbf6f2

…n and dynamically resizing based on element heights using refs

removes moved css from layout module file

8720444

adds padding to box and puts it behind content

42ebf28

layout flow changes from adjusting padding

c2e7e08

resolves type errors and makes final layut adjustment

f45ca32

Merge remote-tracking branch 'origin/sc-2617/dynamic-landing-pages' i…

965bfd3

…nto sc-2617/dynamic-landing-pages

fixes bug where box height didnt change when accordion expands

9ad6dbb

removes a timeout method i was using when debugging

c31081f

sets background box properties to main and adds scroll offset

e31cadd

adjusts background fill to be aligned to more recent designs, and rem…

9d5d978

…oves active styles from in page nav as it is flashing the active state in weird ways

adjusts styling on smaller screen widths

4960daa

Merge remote-tracking branch 'origin/sc-2617/dynamic-landing-pages' i…

fdb22de

…nto sc-2617/dynamic-landing-pages # Conflicts: # src/layout/DefaultLayout/LandingPageLayout.module.scss # src/pages/landing/[landingPage]/index.tsx

Merge branch 'main' into sc-2617/dynamic-landing-pages

66d291b

cleaning up styles and index files after merge

4170f6c

assigns correct styles from figma to the docs accordion

15b3a14

shkeating marked this pull request as ready for review November 16, 2023 19:34

shkeating requested review from a team as code owners November 16, 2023 19:34

shkeating approved these changes Nov 16, 2023

View reviewed changes

AnnaGingle assigned AnnaGingle and unassigned AnnaGingle Nov 16, 2023

gidjin approved these changes Nov 17, 2023

View reviewed changes

mergify bot merged commit 6bd7236 into main Nov 17, 2023

mergify bot deleted the sc-2617/dynamic-landing-pages branch November 17, 2023 01:47

spacecadets-infra mentioned this pull request Nov 17, 2023

chore(main): release 4.25.0 #1142

Merged

jcbcapps mentioned this pull request Nov 17, 2023

test: Add unit tests for [landingPage] #1153

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Dynamic landing pages #1143

feat: Dynamic landing pages #1143

jcbcapps commented Nov 3, 2023 •

edited

Loading

shortcut-integration bot commented Nov 3, 2023

jcbcapps Nov 7, 2023

jcbcapps Nov 8, 2023

jcbcapps Nov 8, 2023

jcbcapps Nov 8, 2023

jcbcapps Nov 8, 2023

jcbcapps Nov 8, 2023

shkeating commented Nov 15, 2023

shkeating left a comment

gidjin left a comment

gidjin Nov 17, 2023

jcbcapps Nov 17, 2023

		// eslint-disable-next-line @typescript-eslint/no-explicit-any
		const pageContent = (): any => {

feat: Dynamic landing pages #1143

feat: Dynamic landing pages #1143

Conversation

jcbcapps commented Nov 3, 2023 • edited Loading

SC-2617

Proposed changes

Reviewer notes

Setup

Start the system

Start storybook

Code review steps

As the original developer, I have

As a reviewer, I have

Screenshots

shortcut-integration bot commented Nov 3, 2023

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

shkeating commented Nov 15, 2023

shkeating left a 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

jcbcapps commented Nov 3, 2023 •

edited

Loading