feat: display course topics status

[gabriele-ct]

https://github.com/commercetools/learning-tech/issues/376

[changeset-bot]

🦋 Changeset detected

Latest commit: 33964e7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@commercetools-docs/gatsby-theme-learning	Minor
@commercetools-docs/gatsby-theme-docs	Minor
@commercetools-website/docs-smoke-test	Minor
@commercetools-website/documentation	Minor
@commercetools-website/api-docs-smoke-test	Patch
@commercetools-website/site-template	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

🚚 Build Process Details

Preview URLs:

• https://docskit-ga-topic-status.commercetools.vercel.app (permalink to the last build of this branch)
• https://docskit-sha-a5853603dac085f84d8535017882fd9b156c4923.commercetools.vercel.app (this commit)

[zbalek]

Looks good! In terms of behaviour though - it seems that I get a green tick even if I don't pass the quiz - are we able to change this (so that I only get a green tick if I answer correctly) or can we only see if a user attempted a quiz? @gabriele-ct

[FFawzy]

@zbalek
that definitely should be the case that you don't get a tick if failed.
from my side I'm getting the correct behaviour

if you want we can huddle or you can write here the steps to reproduce what you saw so I can check it

[nkuehn]

I only found the time to scan it, not fully read it. So some of what caught my eyes might be wrong.

Please add a PR description or link to the ticket so that the definition is known what is in scope and what not.

[nkuehn]

I wonder if a data structure that has defaults might be better suited. I can't really wrap my head around why exactly to be honest, it's just a feeling.

[gabriele-ct]

not sure what you mean here. but the feature flag feature is working and I would descope this discussion from this PR. We can have a discussion about it and then create a separate ticket

[zbalek]

@FFawzy happy to approve then - For me, once I completed the quiz (incorrectly) the 'Overview' page got a green tick

[nkuehn]

I tried the UI now, too.

@gabriele-ct nice to see the SWR approach is working - no flicker, the status in the nav is updating with a little delay but smoothly. What's slightly irritating though is that when the page is reloaded (e.g. after navigating to a different docs page and back) the icon is the same as if I did not do anything yet while it loads. Maybe we should use the localstorage based SWR cache and clear it on login / logout.

@FFawzy I forgot what the logic behind the "overview" page with no content becoming green automatically after being auto-enrolled into the course when first taking the quiz on the second page. If that is a content-only page it would be strange to mark it done but only after you finished the next page. It could either have a neutral look in general because it has nothing to be completed or we provide the API and background listener that marks the activity completed on that page when you scroll down to the end or someething.

[FFawzy]

@nkuehn yea I'd skip the overview page since it is just an intro (at least in the MVP state) .. maybe later when we introduce reading-trackers we can apply it to those pages

WDYT ? @zbalek
removing the circle from the overview pages as there is nothing to be tracked there (yet)

[zbalek]

@FFawzy I think I just got confused, I think it makes sense to mark it as completed once user goes to the next page from the overview page - so let's keep it. I just got confused cause once I completed the quiz, it was the overview page that got the tick instead of the test one. Apologies for the confusion.

[nkuehn]

I just got confused cause once I completed the quiz, it was the overview page that got the tick instead of the test one.

That's effectively the problem I meant. If you complete the quiz on the second page but don't pass it, the first page is completed implicitly (we only check when you submit a quiz) but the second is not.

Learning for me: we may need to have

• an "in progress" state visually for e.g. partially done (failed the quiz)
• and / or a special visual state for pages (topics) that do not have any activity that can have a status

[zbalek]

From my perspective, I wouldn't add additional states as even though they may be factually correct, user does not need to have e.g. 4 different states. Their main goal here would be to understand what's done and what's not done.
My proposal:

• if a page is in progress (.e.g. failed quiz) = not completed = empty circle
• topics that don't have activity = once user visits the page = learning is completed = green tick

[gabriele-ct]

@gabriele-ct nice to see the SWR approach is working - no flicker, the status in the nav is updating with a little delay but smoothly. What's slightly irritating though is that when the page is reloaded (e.g. after navigating to a different docs page and back) the icon is the same as if I did not do anything yet while it loads. Maybe we should use the localstorage based SWR cache and clear it on login / logout.

@nkuehn SWR is doing an excellent job (I admit it's very powerful). My feeling is that avoiding any possible "caching" layer should be our objective, you know better then me that there are possible corner cases that can lead to unwanted behaviour. What I believe the solution should be in this particular case is to handle a loading state UI wise to give the user the feeling that something is happening and it's not actually seeing the "loaded state".

[nkuehn]

avoiding any possible "caching" layer should be our objective,

@gabriele-ct SWR is a cache system in its main functionality, so I don't really understand your concern to be honest. At the moment it's a memory cache with pageload lifetime (which is across page navigations after the react app hydrated). The idea is to extend the lifetime across page reloads. invalidating has to be done anyways after logout, even in the current state.

@zbalek we dont have the technical setup yet to track "page read" - so a different state for pages that have no actions might be a transitional need only. We'll discuss it in the tech daily what is more feasible

[gabriele-ct]

@gabriele-ct SWR is a cache system in its main functionality, so I don't really understand your concern to be honest. At the moment it's a memory cache with pageload lifetime (which is across page navigations after the react app hydrated). The idea is to extend the lifetime across page reloads. invalidating has to be done anyways after logout, even in the current state.

An additional caching layer I mean...
Anyhow, if you feel is needed, let's specify it properly and implement it. Definitely out of this ticket scope

[nkuehn]

An additional caching layer I mean...

Excuse the confusion @gabriele-ct - I did not mean an additional layer, I only thought about configuring SWRs cache backend to use localstorage instead of the default memory store. https://swr.vercel.app/docs/advanced/cache#localstorage-based-persistent-cache

[gabriele-ct]

I did not mean an additional layer, I only thought about configuring SWRs cache backend to use localstorage instead of the default memory store.

ahhh, I thought implementing something on our side. I'll give it a go

[gabriele-ct]

@nkuehn I created a separate branch for testing swr with localstorage
#1604

I'd like you to have a look, maybe try it and let me know what you think. My feeling after spending some time on it, is that it's not that trivial to setup (definitely not just a matter of adding a cache handler) and opens several corner cases that might be hard to figure out. Let me know what you think

[FFawzy]

Zoe gave the OK on the following usecases

• loading state = (after refreshing the page, .. etc, AKA no cache in memory ) show no circles, we reserve the space for it from the beginning to avoid layout shifting and only show the circles once we have a response
• Stale while revalidating use case = showing the old value until its updated
• Overview being checked after first quiz = OK for now, until we implement the reading tracker

---

which means we can apply the localstorage as a cache provider as an enhancement after MVP.
along with the reading tracker

CC: @zbalek @nkuehn @gabriele-ct

[nkuehn]

OK

which means we can apply the localstorage as a cache provider as an enhancement after MVP.
along with the reading tracker
Can be separate, this sounds like they need to go together.

[nkuehn]

There's still a lot of layout shift and multiple times. See video, shows two consecutive page reloads. The vertical scrolling is a different topic, but there are two additional steps of layout shift

• once when the page hydrates, the indentation depth changes from the statically pregenerated version (I guess the indentation is in a code path that only evaluates in the hydrated app - but the information that the page is a learning page is in the frontmatter so it can be applied statically, too)
• then the chapter name also shifts a bit when the data is loaded (I guess that is just polish work - the chapter level status icon seems to be bigger than the space reserved. Maybe rather constrain the icon into the box that reserves the space than letting the icon take whatever space it needs? )

Bildschirmaufnahme.2023-04-17.um.14.58.28.mov

[gabriele-ct]

• once when the page hydrates, the indentation depth changes from the statically pregenerated version (I guess the indentation is in a code path that only evaluates in the hydrated app - but the information that the page is a learning page is in the frontmatter so it can be applied statically, too)

This happens because the page has no knowledge of the logged state of the user and we agreed not reserve the icon space if the user is not logged in. Once the logged state becomes available we can reserve the space while the actual data gets loaded

• then the chapter name also shifts a bit when the data is loaded (I guess that is just polish work - the chapter level status icon seems to be bigger than the space reserved. Maybe rather constrain the icon into the box that reserves the space than letting the icon take whatever space it needs? )

This can be easily fixed, I'll have a look into it

[nkuehn]

This happens because the page has no knowledge of the logged state of the user and we agreed not reserve the icon space if the user is not logged in. Once the logged state becomes available we can reserve the space while the actual data gets loaded

Sorry, this does not make sense to me. We now have a state where we don't show any icon but blank space while the app does not know anything about the user state yet (but knows that the page is a course page).

Currently the app reserves a little less space while the app is hydrating and then a little more when the API is called but in any case there is no icon visible. The user does not care about which exact technical phase the loading is is to make a visual difference.

@zbalek maybe you're more talented in verbalizing the problem I see in the video (or tell me that I'm wrong)?

[gabriele-ct]

Currently the app reserves a little less space while the app is hydrating and then a little more when the API is called but in any case there is no icon visible. The user does not care about which exact technical phase the loading is is to make a visual difference.

@nkuehn I have no problem at all in implementing the initial spacing, I just implemented what was initially in the requirements . I understand the current time is a bit hectic but It would be beneficial in the future, define how the UI should look like in different app states to avoid all these roundtrips around an icon position 👍

[zbalek]

@gabriele-ct I understand your concerns. In this case believe that the outcome that we agreed on (Fady's comment above) was that we reserve the space for the icons from the beginning to avoid layout shifting and only show them once we have a response. @nkuehn So what seems to be happening in the video is that it looks like the layout did just that but then once it loads it actually jumps one more time - which is what we wanted to avoid by reserving the space.

[FFawzy]

🎊

[nkuehn]

❤️