Block based rendering #443

kilemensi · 2023-06-05T13:48:59Z

Description

Our current implementation kinds of cheat: Looks at a page and processes all blocks that it knows will more or less be in a page. This PR flips that process: process all blocks in a given page without caring which page it is.

DO NOT MERGE: This PR is up to kick-start discussions and it's not ready to merge just yet. Pages that currently use block-based rendering include:

Privacy policy,
Knowledge > News, and
About.

Type of change

Bug fix (non-breaking change which fixes an issue)
Chore (non-breaking change that improves code quality)

Screenshots

N/A

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas

vercel · 2023-06-05T13:49:04Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
codeforafrica	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 7, 2023 6:59am

koechkevin

@kilemensi I try to access http://localhost:3000 I get an error

kilemensi · 2023-06-06T07:00:32Z

@kilemensi I try to access http://localhost:3000 I get an error

That's why it's not ready for merging just yet @koechkevin ... Pushed a fix for it.

koechkevin

@kilemensi I have tested this approach and it looks good.

apps/charterafrica/src/lib/data/common/index.js

kilemensi · 2023-06-07T06:22:11Z

@kilemensi I have tested this approach and it looks good.

Aaaight @koechkevin but what are you thoughts? Is this approach better that what we have? Can we improve the implementation, etc?

The idea is to get everyone's input?

saminegash · 2023-06-07T10:02:23Z

apps/charterafrica/src/components/Articles/Articles.js

-  const res = useArticles(slug, { locale, q, sort, page, pageSize });
+  const res = useArticles(collection || slug, {
+    locale,
+    q,


What is this variable q? the other variables represent the actual representation I believe but q is not clear?

q = query e.g. https://www.google.com/search?q=code+for+africa or https://github.com/search?q=code+for+africa&type=repositories

saminegash · 2023-06-07T10:02:27Z

apps/charterafrica/src/components/LongForm/LongForm.js

@@ -33,7 +33,7 @@ const LongForm = React.forwardRef(function LongForm(props, ref) {
        {content.map((c) => {


using plurals for naming lists then while using the map method we can use the singular reference
e.g contents.map((content) => { ...

💯 ... except our CMS has chosen to call the page contents content so we're stuck with that for now.

What CMS are we using?

Payload CMS... Speaking of CMS, I'd also like to hear you views and experiences with various CMSes.

saminegash · 2023-06-07T10:04:16Z

apps/charterafrica/src/lib/data/common/blockify/articles.js

+  if (!docs?.length) {
+    return null;
+  }
+  const tags = docs.flatMap((doc) => doc.tags).filter((tag) => tag?.slug);


using typescript might help a lot with unsafe type issues here.

💯 ... that's the next step: monorepo -> ci/cd -> typescript -> end-to-end testing. Happy to hear your suggestions as well.

you mean we are going to convert our codebase based on this steps

Yes-ish.

We've moved about 3 apps into this monorepo (charter.africa, codeforafrica and promisetracker). We still have a few to go (pesayetu, trolltracker, etc.)

We've already implemented CI/CD for charter.africa and codeforafrica (you can see the workflows in the repo).

We have included Jest (for component testing) and Playwright (for end-to-end testing) but we're not really using them that much (just snapshot testing w/ Jest).

So the idea is come up with a comprehensive plan that will allow us to move everything UI into this repo and make sure we have ci/cd -> typescript -> end-to-end testing for all the apps.

apps/charterafrica/src/lib/data/common/blockify/articles.js

saminegash · 2023-06-07T10:08:37Z

apps/charterafrica/src/lib/data/common/blockify/articles.js

+  const { slug } = page;
+
+  // SWR fallback
+  let swrKey = `/api/v1/knowledge/${slug}`;


base end points like this shall change time to time so it should be added to configuration files

Happy to chat about how we'll do it for all base end points in the app/repo.

kilemensi · 2023-06-07T11:12:50Z

Thanks for the review @saminegash, but what are you thoughts about the approach so far?

saminegash · 2023-06-08T06:50:23Z

I think I cannot say more as I am just looking into the codebases and diving in I believe.

kilemensi · 2023-06-08T12:57:11Z

Your review and thoughts @kelvinkipruto

kilemensi · 2023-06-09T12:42:34Z

@kelvinkipruto

kelvinkipruto · 2023-06-12T14:17:30Z

@kilemensi I have gotten this to work, but it's hard to follow the code flow compared to the current page-based processing. For example, the pagify function means some block will still be processed per page?
I like how blockify works, and I think it's a better way than our current method.
One issue I have faced though is when I add a news block to any that does not have a parent page, it breaks. ( I will investigate further) but given this approach, shouldn't it be processed regardless of the page it is on?

kilemensi · 2023-06-12T14:32:32Z

@kilemensi ... the pagify function means some block will still be processed per page?

pagify is for those pages that do not exist in the CMS @kelvinkipruto. For example single news item page. pagify creates the block(s) that should be displayed in this page but doesn't process/render the page. The processing/rendering still happens in blockify.

Not sure if there is anything we can do about these sort of pages but any ideas are welcomed.

I like how blockify works, and I think it's a better way than our current method.

👍🏽

One issue I have faced though is when I add a news block to any that does not have a parent page, it breaks. ( I will investigate further) but given this approach, shouldn't it be processed regardless of the page it is on?

Yes, news block should be processed regardless of which page it's on. If it doesn't work, do investigate and report. It's most likely an edge case we haven't considered.

kilemensi · 2023-07-13T14:05:49Z

Taking this back to draft to focus on current PROD issues.

kilemensi added 16 commits May 29, 2023 16:21

Introduce block-based processing

5091b72

Remove simplest pages first

84b13a0

Add Explainers block

9f49b43

Add to block rendering in processPage

18ceb91

Update explainers slug

5dc5965

Make featuredPost a resuable field

898ee62

Add News block to drive how news article are processed from CMS

cb5a7ca

Avoid block slug duplication

764f96b

Show News block in CMS pages

971a8ac

Process page data block by block

be18875

For non-page items, prepare pages before processing blocks

360b042

Load page rather than hard-coding it

b12d7ae

Process news via blocks

103690b

Prefer collection prop when available

c10f637

Show new our-news block on front-end

b60e39e

Merge branch 'main' into chore/charterafrica_blockbasedrendering

9570eb8

kilemensi added bug Something isn't working chore A task that needs to be done (neither enhancement or bug) labels Jun 5, 2023

kilemensi self-assigned this Jun 5, 2023

kilemensi requested review from koechkevin and kelvinkipruto June 5, 2023 13:50

Fix post block return structure

a8577a3

vercel bot deployed to Preview June 5, 2023 13:56 View deployment

koechkevin reviewed Jun 6, 2023

View reviewed changes

Homepage doesn't have slugs

d2e8708

vercel bot deployed to Preview June 6, 2023 07:02 View deployment

kilemensi requested a review from koechkevin June 6, 2023 11:51

koechkevin reviewed Jun 6, 2023

View reviewed changes

apps/charterafrica/src/lib/data/common/index.js Show resolved Hide resolved

Handle when item page doesn't exist

b915556

vercel bot deployed to Preview June 7, 2023 06:22 View deployment

saminegash suggested changes Jun 7, 2023

View reviewed changes

Merge branch 'main' into chore/charterafrica_blockbasedrendering

1ddc447

vercel bot deployed to Preview July 7, 2023 06:59 View deployment

kilemensi marked this pull request as draft July 13, 2023 14:08

This was referenced Jul 20, 2023

@/codeforafrica global Footer #508

Closed

@/codeforafrica global Navigation #509

Closed

@/codeforafrica collection Posts #510

Closed

This was referenced Aug 31, 2023

@/codeforafrica Hero Block #560

Closed

@/codeforafrica Projects #561

Closed

@/codeforafrica seo #562

Open

@/codeforafrica partners #563

Closed

@/codeforafrica impact #564

Closed

This was referenced Sep 19, 2023

@codeforafrica Stories #592

Closed

@codeforafrica opportunities #593

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block based rendering #443

Block based rendering #443

kilemensi commented Jun 5, 2023 •

edited

Loading

vercel bot commented Jun 5, 2023 •

edited

Loading

koechkevin left a comment

kilemensi commented Jun 6, 2023

koechkevin left a comment

kilemensi commented Jun 7, 2023

saminegash Jun 7, 2023

kilemensi Jun 7, 2023

saminegash Jun 7, 2023

kilemensi Jun 7, 2023

saminegash Jun 8, 2023

kilemensi Jun 8, 2023

saminegash Jun 7, 2023

kilemensi Jun 7, 2023

saminegash Jun 8, 2023

kilemensi Jun 8, 2023

saminegash Jun 7, 2023

kilemensi Jun 7, 2023

kilemensi commented Jun 7, 2023

saminegash commented Jun 8, 2023

kilemensi commented Jun 8, 2023

kilemensi commented Jun 9, 2023

kelvinkipruto commented Jun 12, 2023

kilemensi commented Jun 12, 2023

kilemensi commented Jul 13, 2023

		@@ -33,7 +33,7 @@ const LongForm = React.forwardRef(function LongForm(props, ref) {
		{content.map((c) => {

Block based rendering #443

Are you sure you want to change the base?

Block based rendering #443

Conversation

kilemensi commented Jun 5, 2023 • edited Loading

Description

Type of change

Screenshots

Checklist:

vercel bot commented Jun 5, 2023 • edited Loading

koechkevin left a comment

Choose a reason for hiding this comment

kilemensi commented Jun 6, 2023

koechkevin left a comment

Choose a reason for hiding this comment

kilemensi commented Jun 7, 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

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

kilemensi commented Jun 7, 2023

saminegash commented Jun 8, 2023

kilemensi commented Jun 8, 2023

kilemensi commented Jun 9, 2023

kelvinkipruto commented Jun 12, 2023

kilemensi commented Jun 12, 2023

kilemensi commented Jul 13, 2023

kilemensi commented Jun 5, 2023 •

edited

Loading

vercel bot commented Jun 5, 2023 •

edited

Loading