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.

Sign up for GitHub

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

Jump to bottom

feat(preview): add preview for new homepage variant #1485

Merged
merged 11 commits into from
Sep 20, 2023
Merged

feat(preview): add preview for new homepage variant #1485

merged 11 commits into from
Sep 20, 2023

Conversation

seaerchin
Copy link
Contributor

@seaerchin seaerchin commented Sep 8, 2023 •
edited
Loading

Problem

This adds a new homepage variant without the image.

Solution

  1. break down TemplateHeroSection -> use diff components, so that it's easier to use/maintain
  2. convert template from .js to .ts -> PropTypes already done, so not much effort
  3. for template, i copied over the styling from the template repo as-is and used .css to do this to conform to the style of handling in the template repository.

Screenshots

Recording 2023-09-08 at 14 38 01

Tests

This is image only variant

  • go to the homepage
  • click on hero
  • change the variant from center to image only
  • the preview should update
  • there should be only the button + highlight
  • updating highlights should also update preview (title, add highlights, reorder)
  • drop-down added in later PR but if it's also in then
  • update to drop-down interaction
  • preview should also update
  • change the dropdown (title, add dropdown, reorder)
  • the preview should also update

@seaerchin seaerchin requested a review from a team September 8, 2023 03:53
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 04:05 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 04:07 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 04:40 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 04:42 — with GitHub Actions Inactive
@seaerchin seaerchin changed the base branch from develop to feat/is-450-hero-banner-panel September 8, 2023 04:52
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 05:06 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 05:07 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 05:52 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 06:50 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 06:51 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 07:31 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 09:08 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 09:09 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 8, 2023 14:43 — with GitHub Actions Inactive
@seaerchin seaerchin requested review from dcshzj, a team and alexanderleegs September 13, 2023 06:20
@seaerchin seaerchin temporarily deployed to staging September 20, 2023 03:03 — with GitHub Actions Inactive
@seaerchin seaerchin temporarily deployed to staging September 20, 2023 03:21 — with GitHub Actions Inactive
alexanderleegs
alexanderleegs reviewed Sep 20, 2023
View reviewed changes
src/templates/homepage/HeroSection/HeroSection.tsx
Comment on lines +141 to +149
{variant === "center" ? (
<HeroCenteredLayout
hero={hero}
dropdownIsActive={dropdownIsActive}
toggleDropdown={toggleDropdown}
/>
) : (
<HeroImageOnlyLayout />
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a fallback for when variant doesn't exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll raise a PR at the end so that i can account for all the variants!

@seaerchin seaerchin force-pushed the feat/is-450-hero-banner-panel branch from d55f14e to 4ce3229 Compare September 20, 2023 04:40
@seaerchin seaerchin requested review from alexanderleegs and a team September 20, 2023 05:17
@seaerchin seaerchin temporarily deployed to staging September 20, 2023 05:30 — with GitHub Actions Inactive
alexanderleegs
alexanderleegs approved these changes Sep 20, 2023
View reviewed changes
Copy link
Contributor

@alexanderleegs alexanderleegs left a comment

Choose a reason for hiding this comment

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

lgtm

Base automatically changed from feat/is-450-hero-banner-panel to develop September 20, 2023 08:46
@seaerchin seaerchin merged commit 178edae into develop Sep 20, 2023
3 checks passed
@seaerchin seaerchin deleted the feat/is-451-preview branch September 20, 2023 08:49
@seaerchin seaerchin temporarily deployed to staging September 20, 2023 09:02 — with GitHub Actions Inactive
@harishv7 harishv7 mentioned this pull request Sep 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcshzj dcshzj dcshzj approved these changes

@alexanderleegs alexanderleegs alexanderleegs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@seaerchin @alexanderleegs @dcshzj