Skip to content
This repository has been archived by the owner on Jun 2, 2022. It is now read-only.

feat: sections #371

Merged
merged 5 commits into from
Mar 11, 2022
Merged

feat: sections #371

merged 5 commits into from
Mar 11, 2022

Conversation

tlgimenes
Copy link
Contributor

@tlgimenes tlgimenes commented Mar 4, 2022

What's the purpose of this pull request?

The whole idea of sections is to split app logic into replaceable/reusable strips of screen content so that CMS users can easily replace/rearrange the desired component on the screen. To allow CMS users to change the page's section, prismic and other CMSs usually use the following Render component logic:

interface Section {
    name: string
    props: any
}

const Sections = {
   Hero: lazy(() => import('./sections/Hero'))
   BannerText: lazy(() => import('./sections/BannerText'))
   ProductShelf: lazy(() => import('./sections/ProductShelf'))
   // ... list goes on an on
}

function Render ({ sections }: { sections: Section[]}) {
  return (
    <>
      {sections.map(section => {
          const Component = Sections[section.name]
          
          return (
             <Suspense fallback={null}>
                <Component {...section.props} />
             <Suspense>
          )
      })}
    </>
  )     
}

This means that the Section map has to render a self contained section.

This PR moves all external logic that were hard coded into the /pages components into the sections so CMS users can correctly change and re-order the sections

How to test it?

Nothing should have changed

Checklist

  • CHANGELOG entry added

@netlify
Copy link

netlify bot commented Mar 4, 2022

✔️ Deploy Preview for basestore ready!

🔨 Explore the source changes: dd69d79

🔍 Inspect the deploy log: https://app.netlify.com/sites/basestore/deploys/622b92d1389edd0008da7931

😎 Browse the preview: https://deploy-preview-371--basestore.netlify.app

@gatsby-cloud
Copy link

gatsby-cloud bot commented Mar 4, 2022

Gatsby Cloud Build Report

basestore

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 8m

Performance

Lighthouse report

Metric Score
Performance 💚 94
Accessibility 💚 100
Best Practices 💚 100
SEO 💚 93

🔗 View full report

@vtex-sites
Copy link

vtex-sites bot commented Mar 4, 2022

Preview is ready

This pull request generated a Preview

👀   Preview: https://preview-371--base.preview.vtex.app
🔬   Go deeper by inspecting the Build Logs
📝   based on commit 0471fcb

@tlgimenes tlgimenes marked this pull request as ready for review March 7, 2022 13:10
@igorbrasileiro
Copy link
Contributor

Why this PR is not here @vtex-sites/storeframework.store ?

@tlgimenes
Copy link
Contributor Author

Why this PR is not here @vtex-sites/storeframework.store ?

Because I want to have the less difference between this repo and storeframework.store. Without the need for CMS, the sections folder of this repo may be removed too

@tlgimenes tlgimenes requested a review from a team March 8, 2022 18:05
Copy link
Contributor

@filipewl filipewl left a comment

Choose a reason for hiding this comment

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

LGTM! Found no visual regression on the main pages and the final organization on pages/* looks like what's expected (with only sections).

@filipewl filipewl requested a review from a team March 8, 2022 18:35
Copy link
Member

@eduardoformiga eduardoformiga left a comment

Choose a reason for hiding this comment

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

Nice Job! 🎉
should we remove the Section from the search page as well?

@@ -78,7 +78,7 @@ function GalleryPage({
/>
<div className="product-listing__results-sponsored">
<h3>Sponsored</h3>
<ProductTiles products={productsSponsored.slice(0, 2)} />
<ProductTiles products={productsSponsored.slice(0, 2)} title="" />
Copy link
Member

Choose a reason for hiding this comment

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

Should we make the title prop optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. This component is weird because it's using another section. Sections should be self contained and not depend on other sections. This should be refactored on another PR. I'll add a TODO for this. Thanks!

@tlgimenes
Copy link
Contributor Author

tlgimenes commented Mar 11, 2022

Nice Job! 🎉
should we remove the Section from the search page as well?

Nice! I haven't seen that! I'll change it

@tlgimenes tlgimenes merged commit 0471fcb into master Mar 11, 2022
@tlgimenes tlgimenes deleted the feat/sections branch March 11, 2022 18:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants