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

feat: Add hero image support for landing page #1189

Merged
merged 10 commits into from
Jan 13, 2024
Merged

Conversation

jcbcapps
Copy link
Contributor

@jcbcapps jcbcapps commented Jan 3, 2024

SC-2636

Proposed changes

Should be merged after https://github.com/USSF-ORBIT/ussf-portal-cms/pull/417

  • Update query to include hero image for landing pages
  • Display hero image on landing page

Reviewer notes

You will need the accompanying CMS branch in order for this to work.

Testing is straightforward:

  • Create a new landing page and add a hero image
  • Navigate to the page in the portal and you should see your image

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

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

Copy link

This pull request has been linked to Shortcut Story #2636: Image and video support for landing page hero or badge.

@jcbcapps jcbcapps marked this pull request as ready for review January 8, 2024 18:06
@jcbcapps jcbcapps requested review from a team as code owners January 8, 2024 18:06
@jbecker01
Copy link
Contributor

Looks good!

Copy link
Contributor

@shkeating shkeating left a comment

Choose a reason for hiding this comment

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

lookin good!


// eslint-disable-next-line @typescript-eslint/no-explicit-any
const pageContent = (): any => {
return (
<>
<h1 className={styles.pageTitle}>{pageTitle}</h1>
<div className={styles.pageTitleContainer}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Thought(non-blocking): I wonder if we should refactor this so that most of pageContent function is in a separate component so that we can display it in storybook. I don't think this needs to happen now though since the page is already not in storybook. Probably we should come to some sort of consensus on this type of thing, maybe next design eng sync.

@abbyoung abbyoung merged commit 141a348 into main Jan 13, 2024
12 of 14 checks passed
@abbyoung abbyoung deleted the sc-2636/lp-images branch January 13, 2024 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants