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

Ga topic status with localstorage #1604

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
8 changes: 8 additions & 0 deletions .changeset/chilled-houses-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@commercetools-docs/gatsby-theme-learning': minor
'@commercetools-docs/gatsby-theme-docs': minor
'@commercetools-website/docs-smoke-test': minor
'@commercetools-website/documentation': minor
---

Added status indicator for course and course topics
11 changes: 9 additions & 2 deletions packages/gatsby-theme-docs/gatsby-node.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ export const createSchemaCustomization = ({ actions, schema }) => {
}),
},
courseId: { type: 'Int' },
topicName: { type: 'String' },
},
interfaces: ['Node'],
}),
Expand Down Expand Up @@ -352,6 +353,9 @@ export const onCreateNode = async (
courseId: node.frontmatter.courseId
? Number(node.frontmatter.courseId)
: null,
topicName: node.frontmatter.topicName
? String(node.frontmatter.topicName)
: null,
};

actions.createNode({
Expand Down Expand Up @@ -410,7 +414,8 @@ async function createContentPages(
allContentPage {
nodes {
slug
courseId
courseId,
topicName,
}
}
allReleaseNotePage(sort: { date: DESC }) {
Expand Down Expand Up @@ -447,7 +452,7 @@ async function createContentPages(
(pageLinks, node) => [...pageLinks, ...(node.pages || [])],
[]
);
pages.forEach(({ slug, courseId }) => {
pages.forEach(({ slug, courseId, topicName }) => {
const matchingNavigationPage = navigationPages.find(
(page) => trimTrailingSlash(page.path) === trimTrailingSlash(slug)
);
Expand Down Expand Up @@ -492,6 +497,8 @@ async function createContentPages(
}
if (courseId) {
contentPageData = {...contentPageData, context: {...contentPageData.context, courseId}}
}if (topicName) {
contentPageData = {...contentPageData, context: {...contentPageData.context, topicName}}
}
actions.createPage(contentPageData);

Expand Down
25 changes: 21 additions & 4 deletions packages/gatsby-theme-docs/src/hooks/use-course-pages.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const useCoursePages = () => {
nodes {
slug
courseId
topicName
}
}
}
Expand All @@ -25,17 +26,33 @@ var isIndexed = false;
const courseMapToPages = (coursePagesData) => {
if (!isIndexed) {
coursePagesData.forEach((element) => {
coursePageMap.set(element.slug, { courseId: element.courseId });
const { courseId, topicName } = element;
coursePageMap.set(element.slug, { courseId, topicName });
});
}
isIndexed = true;
};

/**
* Given a page slug, it returns the courseId where the page belongs
* Given an array of page slugs, it returns an array with course info
* if exists, otherwise undefined
*/
export const useCourseInfoByPageSlug = (pageSlug) => {
export const useCourseInfoByPageSlugs = (pageSlugs) => {
courseMapToPages(useCoursePages());
return coursePageMap.get(pageSlug);
const courseInfo = pageSlugs.reduce(
(prev, curr) => ({ ...prev, [curr]: coursePageMap.get(curr) }),
{}
);
// sanity check: all the pages should belong to the same course, therefore have the same courseId
const isOk = Object.values(courseInfo)?.every(
(info, _, theArray) => info?.courseId === theArray[0]?.courseId
);
if (!isOk) {
// TODO: decide if we want to make this blocker
console.warn(
'pages belonging to the same course have different courseId metadata',
courseInfo
);
}
return courseInfo;
};
4 changes: 0 additions & 4 deletions packages/gatsby-theme-docs/src/layouts/content.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import LayoutPageNavigation from './internals/layout-page-navigation';
import LayoutPageContent from './internals/layout-page-content';
import PageContentInset from './internals/page-content-inset';
import PageReadTime from './internals/page-read-time-estimation';
import { PageCourseStatus } from '@commercetools-docs/gatsby-theme-learning';

const LayoutContent = (props) => {
const { ref, inView, entry } = useInView();
Expand Down Expand Up @@ -82,9 +81,6 @@ const LayoutContent = (props) => {
{props.pageData.showTimeToRead && (
<PageReadTime data={props.pageData} />
)}
{props.pageContext.courseId && (
<PageCourseStatus courseId={props.pageContext.courseId} />
)}
</LayoutPageHeader>
<LayoutPageHeaderSide>
<SpacingsStack scale="m">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React from 'react';
import { SWRConfig } from 'swr';
import PropTypes from 'prop-types';
import styled from '@emotion/styled';
import { designSystem } from '@commercetools-docs/ui-kit';
import useIsClientSide from '../../hooks/use-is-client-side';

/* NOTE: `overflow` shorthand is only supported is Chrome and FF */
const Root = styled.div`
Expand Down Expand Up @@ -41,18 +43,55 @@ const Container = styled.div`
}
`;

const LayoutApplication = (props) => (
<>
<Root
role="application"
id="application"
isGlobalNotificationVisible={Boolean(props.globalNotification)}
>
<Container {...props} />
</Root>
<div id="modal-portal" />
</>
);
const localStorageProvider = () => {
// When initializing, we restore the data from `localStorage` into a map.
const map = new Map(JSON.parse(localStorage.getItem('app-cache') || '[]'));

const handleBeforUnload = (event) => {
const appCache = JSON.stringify(Array.from(map.entries()));
Copy link
Contributor Author

@gabriele-ct gabriele-ct Apr 5, 2023

Choose a reason for hiding this comment

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

This handler is rather blind approach as it will simply dump swr state to the cache each time the page is refreshed. But we refresh the page also each time we login and logout.
This is the reason of the 2 custom events that basically disable writing the cache upon login and logout.

localStorage.setItem('app-cache', appCache);
};

const handleBeforeAuth0LoginRedirect = (event) => {
// I'm about to redirect to auth0, I want to disable the set cache logic
window.removeEventListener(handleBeforUnload);
// and ensure the cache is clean
localStorage.removeItem('app-cache');
};

// Before unloading the app, we write back all the data into `localStorage`.
window.addEventListener('beforeunload', handleBeforUnload);

window.addEventListener(
'beforeAuth0LoginRedirect',
handleBeforeAuth0LoginRedirect
);

// We still use the map for write & read for performance.
return map;
};

const LayoutApplication = (props) => {
const { isClientSide } = useIsClientSide();
return (
<>
<Root
role="application"
id="application"
isGlobalNotificationVisible={Boolean(props.globalNotification)}
>
{isClientSide ? (
<SWRConfig value={{ provider: localStorageProvider }}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

First doubt. If we want to have the course/topic status available in the sidebar and (potentially in the future) in the actual content page, the only place common between these 2 layouts is the application layout.
Adding cache provider here means that all the usages of swr in the app will be cached in local cache. This might be what we actually want, or might be creating side effects.

Copy link
Collaborator

Choose a reason for hiding this comment

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

#1 I wonder if something like the SWRConfig would not be better placed in gatsby-browser's "wrapXXXElement" hooks. That implicitly makes them client side only and also makes sure they are in all layouts.

#2 having one cache provider for any SWR even across microsites felt weird to me too in the beginning but I concluded that it's the whole point of it. The cache keys are the URLs you use to fetch from SWR and the content is the API response. And we want that information to be available at least in stale state for other pages / views / microsites. So it must be one fixed key like "app-cache" and also at top level for the domain. We could use a more specific localstorage key maybe that is more explanatory.

<Container {...props} />
</SWRConfig>
) : (
<Container {...props} />
)}
</Root>
<div id="modal-portal" />
</>
);
};
LayoutApplication.propTypes = {
globalNotification: PropTypes.shape({
notificationType: PropTypes.oneOf(['info', 'warning']).isRequired,
Expand Down
101 changes: 80 additions & 21 deletions packages/gatsby-theme-docs/src/layouts/internals/sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ import SiteIcon from '../../overrides/site-icon';
import useScrollPosition from '../../hooks/use-scroll-position';
import { BetaFlag } from '../../components';
import LayoutHeaderLogo from './layout-header-logo';
import { useCourseInfoByPageSlug } from '../../hooks/use-course-pages';
import { PageCourseStatus } from '@commercetools-docs/gatsby-theme-learning';
import { useCourseInfoByPageSlugs } from '../../hooks/use-course-pages';
import {
SidebarCourseStatus,
SidebarTopicStatus,
} from '@commercetools-docs/gatsby-theme-learning';

const ReleaseNotesIcon = createStyledIcon(Icons.ReleaseNotesSvgIcon);

Expand Down Expand Up @@ -91,6 +94,19 @@ const LinkItem = styled.div`
display: flex;
flex-direction: row;
align-items: flex-end;
vertical-align: middle;
`;
const LinkItemWithIcon = styled.div`
padding: 0 0 0 ${designSystem.dimensions.spacings.m};
display: flex;
flex-direction: row;
vertical-align: middle;
svg {
margin-right: 2px;
}
div {
line-height: ${designSystem.typography.lineHeights.cardSmallTitle};
}
`;
const linkStyles = css`
border-left: ${designSystem.dimensions.spacings.xs} solid
Expand Down Expand Up @@ -121,6 +137,24 @@ const activeLinkStyles = css`
color: ${designSystem.colors.light.linkNavigation} !important;
`;

const StatusIconWrapper = styled.span`
display: flex;
vertical-align: middle;
padding-left: 10px; // change this setting to remove the indentation
svg {
margin-right: 5px;
}
`;

const LinkSubtitleWithIcon = (props) => (
<LinkSubtitle>
<StatusIconWrapper>{props.children}</StatusIconWrapper>
</LinkSubtitle>
);
LinkSubtitleWithIcon.propTypes = {
children: PropTypes.any,
};

const SidebarLink = (props) => {
const { locationPath, customStyles, customActiveStyles, ...forwardProps } =
props;
Expand Down Expand Up @@ -234,7 +268,10 @@ SidebarLinkWrapper.propTypes = {
};

const SidebarChapter = (props) => {
const courseInfo = useCourseInfoByPageSlug(props.chapter.pages[0].path);
const courseInfo = useCourseInfoByPageSlugs(
props.chapter.pages.map((page) => page.path)
);
const courseId = Object.values(courseInfo)[0]?.courseId;
const elemId = `sidebar-chapter-${props.index}`;
const getChapterDOMElement = React.useCallback(
() => document.getElementById(elemId),
Expand All @@ -244,26 +281,48 @@ const SidebarChapter = (props) => {
return (
<div role="sidebar-chapter" id={elemId}>
<SpacingsStack scale="s">
<LinkItem>
<LinkTitle>{props.chapter.chapterTitle}</LinkTitle>
{courseInfo?.courseId && (
<PageCourseStatus courseId={courseInfo.courseId} />
)}
</LinkItem>
{courseId ? (
<LinkItemWithIcon>
<SidebarCourseStatus courseId={courseId} />
<LinkTitle>{props.chapter.chapterTitle}</LinkTitle>
</LinkItemWithIcon>
) : (
<LinkItem>
<LinkTitle>{props.chapter.chapterTitle}</LinkTitle>
</LinkItem>
)}

<SpacingsStack scale="s">
{props.chapter.pages &&
props.chapter.pages.map((pageLink, pageIndex) => (
<SidebarLinkWrapper
key={`${props.index}-${pageIndex}-${pageLink.path}`}
to={pageLink.path}
onClick={props.onLinkClick}
location={props.location}
nextScrollPosition={props.nextScrollPosition}
getChapterDOMElement={getChapterDOMElement}
>
<LinkSubtitle>{pageLink.title}</LinkSubtitle>
</SidebarLinkWrapper>
))}
props.chapter.pages.map((pageLink, pageIndex) => {
const currTopicName = courseInfo[pageLink.path]?.topicName;
const TopicIcon =
courseId && currTopicName ? (
<SidebarTopicStatus
courseId={courseId}
pageTitle={currTopicName}
/>
) : null;
return (
<SidebarLinkWrapper
key={`${props.index}-${pageIndex}-${pageLink.path}`}
to={pageLink.path}
onClick={props.onLinkClick}
location={props.location}
nextScrollPosition={props.nextScrollPosition}
getChapterDOMElement={getChapterDOMElement}
>
{TopicIcon ? (
<LinkSubtitleWithIcon>
{TopicIcon}
{pageLink.title}
</LinkSubtitleWithIcon>
) : (
<LinkSubtitle>{pageLink.title}</LinkSubtitle>
)}
</SidebarLinkWrapper>
);
})}
</SpacingsStack>
</SpacingsStack>
</div>
Expand Down
4 changes: 2 additions & 2 deletions packages/gatsby-theme-learning/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ Works against the learning-api.

- `auth0Domain`: the auth0 application domain url (it is defined in the auth0 management app)
- `learnApiBaseUrl`: the learn API base url. It can be omitted if the host running the site matches the api host.
- `features`: a list of feature flags (boolean values) to enable/disable specific functionalities
- `courseStatusIndicator`: feature flag to toggle the course status indicator.
- `features`: an array of strings representing feature flags used to enable/disable specific functionalities. Expected values:
- `status-indicator`: feature flag to toggle the course and topics status indicator.

In order to enable the plugin, at least the following configuration should be added to the `gatsby-config.js` plugin section:

Expand Down
5 changes: 1 addition & 4 deletions packages/gatsby-theme-learning/gatsby-browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ export const wrapRootElement = ({ element }, pluginOptions) => {
value={{
learnApiBaseUrl: pluginOptions.learnApiBaseUrl,
auth0Domain: pluginOptions.auth0Domain,
features: {
courseStatusIndicator:
pluginOptions?.features?.courseStatusIndicator || false,
},
features: pluginOptions?.features || [],
}}
>
{element}
Expand Down
4 changes: 1 addition & 3 deletions packages/gatsby-theme-learning/gatsby-node.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ export const pluginOptionsSchema = ({ Joi }) => {
.allow('')
.default('')
.description(`Learn API base url`),
features: Joi.object({
courseStatusIndicator: Joi.boolean().default(false),
})
features: Joi.array().items(Joi.string()),
});
};
15 changes: 15 additions & 0 deletions packages/gatsby-theme-learning/gatsby-ssr.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import ConfigContext from './src/components/config-context';

export const wrapRootElement = ({ element }, pluginOptions) => {
return (
<ConfigContext.Provider
value={{
learnApiBaseUrl: pluginOptions.learnApiBaseUrl,
auth0Domain: pluginOptions.auth0Domain,
features: pluginOptions?.features || [],
}}
>
{element}
</ConfigContext.Provider>
);
};
1 change: 1 addition & 0 deletions packages/gatsby-theme-learning/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export {
useFetchCourses,
getCourseStatusByCourseId,
} from './src/hooks/use-course-status';
export { useFetchCourseDetails } from './src/hooks/use-course-details';
Loading