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

DOP-4967: Contextualize headings in method selector for "On This Page" list #1237

Merged
merged 8 commits into from
Sep 20, 2024
Merged

DOP-4967: Contextualize headings in method selector for "On This Page" list #1237

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
25fbbdf
reflected
mayaraman19 Sep 16, 2024
46f5f69
merge conflicts
mayaraman19 Sep 16, 2024
65b09e1
address feedback
mayaraman19 Sep 18, 2024
590133a
section depth collapsible
mayaraman19 Sep 18, 2024
9474e78
snapshots
mayaraman19 Sep 18, 2024
49b9307
h2
mayaraman19 Sep 19, 2024
7a5fa17
test
mayaraman19 Sep 19, 2024
c95f893
Merge branch 'main' into DOP-4967
mayaraman19 Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/components/Collapsible/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import Heading from '../Heading';
import { collapsibleStyle, headerContainerStyle, headerStyle, iconStyle, innerContentStyle } from './styles';
import './styles.css';

const Collapsible = ({ nodeData: { children, options }, ...rest }) => {
const Collapsible = ({ nodeData: { children, options }, sectionDepth, ...rest }) => {
mayaraman19 marked this conversation as resolved.
Show resolved Hide resolved
const { id, heading, sub_heading: subHeading } = options;
const { hash } = useLocation();

Expand Down Expand Up @@ -68,7 +68,8 @@ const Collapsible = ({ nodeData: { children, options }, ...rest }) => {
<Box className={cx('collapsible', collapsibleStyle)}>
<Box className={cx(headerContainerStyle)}>
<Box>
<Heading className={cx(headerStyle)} sectionDepth={2} nodeData={headingNodeData}>
{/* Adding 1 to reflect logic in parser, but want to show up as H2 for SEO reasons */}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for this comment! I was a bit confused why we would want to render as an h2 with h3 styling if a writer is placing it under an h2 😕

<Heading className={cx(headerStyle)} sectionDepth={sectionDepth + 1} as={2} nodeData={headingNodeData}>
{heading}
</Heading>
<Body baseFontSize={13}>{subHeading}</Body>
Expand All @@ -79,7 +80,7 @@ const Collapsible = ({ nodeData: { children, options }, ...rest }) => {
</Box>
<Box className={cx(innerContentStyle(open))}>
{children.map((c, i) => (
<ComponentFactory nodeData={c} key={i} {...rest}></ComponentFactory>
<ComponentFactory nodeData={c} key={i} sectionDepth={sectionDepth} {...rest}></ComponentFactory>
))}
</Box>
</Box>
Expand Down
10 changes: 8 additions & 2 deletions src/components/Contents/contents-context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,29 @@
import React from 'react';
import React, { useState } from 'react';
import useActiveHeading from '../../hooks/useActiveHeading';
import useSnootyMetadata from '../../utils/use-snooty-metadata';

const defaultContextValue = {
activeHeadingId: null,
headingNodes: [],
showContentsComponent: true,
activeSelectorId: null,
setActiveSelectorId: () => {},
};

const ContentsContext = React.createContext(defaultContextValue);

const ContentsProvider = ({ children, headingNodes = [] }) => {
const activeHeadingId = useActiveHeading(headingNodes);
const [activeSelectorId, setActiveSelectorId] = useState(null);

const { project } = useSnootyMetadata();
// The guides site is the only site that takes advantage of headings, but never uses the Contents component
const showContentsComponent = project !== 'guides';

return (
<ContentsContext.Provider value={{ activeHeadingId, headingNodes, showContentsComponent }}>
<ContentsContext.Provider
value={{ activeHeadingId, headingNodes, showContentsComponent, activeSelectorId, setActiveSelectorId }}
>
{children}
</ContentsContext.Provider>
);
Expand Down
11 changes: 8 additions & 3 deletions src/components/Contents/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ const formatTextOptions = {
};

const Contents = ({ className }) => {
const { activeHeadingId, headingNodes, showContentsComponent } = useContext(ContentsContext);
const { activeHeadingId, headingNodes, showContentsComponent, activeSelectorId } = useContext(ContentsContext);

if (headingNodes.length === 0 || !showContentsComponent) {
// Don't filter if selector_id is null/undefined
const filteredNodes = headingNodes.filter(
(headingNode) => !headingNode.selector_id || headingNode.selector_id === activeSelectorId
);

if (filteredNodes.length === 0 || !showContentsComponent) {
return null;
}

Expand All @@ -21,7 +26,7 @@ const Contents = ({ className }) => {
return (
<div className={className}>
<ContentsList label={label}>
{headingNodes.map(({ depth, id, title }) => {
{filteredNodes.map(({ depth, id, title }) => {
// Depth of heading nodes refers to their depth in the AST
const listItemDepth = Math.max(depth - 2, 0);
return (
Expand Down
6 changes: 4 additions & 2 deletions src/components/Heading.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ const determineHeading = (sectionDepth) => {
return Body; // use weight=medium prop to style appropriately
};

const Heading = ({ sectionDepth, nodeData, className, ...rest }) => {
const Heading = ({ sectionDepth, nodeData, className, as, ...rest }) => {
const id = nodeData.id || '';
const HeadingTag = determineHeading(sectionDepth);
const asHeading = sectionDepth >= 1 && sectionDepth <= 6 ? `h${sectionDepth}` : 'h6';
const asHeadingNumber = as ?? sectionDepth;
const asHeading = asHeadingNumber >= 1 && asHeadingNumber <= 6 ? `h${asHeadingNumber}` : 'h6';
const isPageTitle = sectionDepth === 1;
const { isMobile, isTabletOrMobile } = useScreenSize();
const { selectors } = useContext(TabContext);
Expand Down Expand Up @@ -140,6 +141,7 @@ Heading.propTypes = {
id: PropTypes.string.isRequired,
}).isRequired,
isProductLanding: PropTypes.bool,
as: PropTypes.number,
};

export default Heading;
8 changes: 7 additions & 1 deletion src/components/MethodSelector/MethodSelector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React, { useEffect, useState } from 'react';
import React, { useEffect, useState, useContext } from 'react';
import { RadioBox, RadioBoxGroup } from '@leafygreen-ui/radio-box-group';
import { css, cx } from '@leafygreen-ui/emotion';
import { palette } from '@leafygreen-ui/palette';
import { theme } from '../../theme/docsTheme';
import { getLocalValue, setLocalValue } from '../../utils/browser-storage';
import { reportAnalytics } from '../../utils/report-analytics';
import { ContentsContext } from '../Contents/contents-context';
import MethodOptionContent from './MethodOptionContent';

const STORAGE_KEY = 'methodSelectorId';
Expand Down Expand Up @@ -104,6 +105,7 @@ const hrStyle = css`
const MethodSelector = ({ nodeData: { children } }) => {
const optionCount = children.length;
const [selectedMethod, setSelectedMethod] = useState(children[0]?.options?.id);
const { setActiveSelectorId } = useContext(ContentsContext);
const [selectedIdx, setSelectedIdx] = useState(0);

// Load method ID saved from last session, if applicable.
Expand All @@ -120,6 +122,10 @@ const MethodSelector = ({ nodeData: { children } }) => {
}
}, [children]);

useEffect(() => {
setActiveSelectorId(selectedMethod);
}, [selectedMethod, setActiveSelectorId]);

return (
<>
<div className={optionsContainer}>
Expand Down
12 changes: 8 additions & 4 deletions tests/unit/__snapshots__/Collapsible.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,21 @@ exports[`collapsible component renders all the content in the options and childr
margin: unset;
font-family: 'Euclid Circular A','Helvetica Neue',Helvetica,Arial,sans-serif;
color: #001E2B;
font-size: 24px;
line-height: 32px;
font-weight: 500;
font-size: 13px;
line-height: 20px;
color: #001E2B;
font-weight: 500;
margin-top: 24px;
margin-bottom: 8px;
color: var(--font-color-primary);
margin-top: 0;
}

.emotion-2 strong,
.emotion-2 b {
font-weight: 700;
}

.emotion-3 {
-webkit-align-self: center;
-ms-flex-item-align: center;
Expand Down Expand Up @@ -509,7 +514,6 @@ exports[`collapsible component renders all the content in the options and childr
<div>
<h2
class="contains-headerlink emotion-2"
weight="medium"
>
This is a heading
<a
Expand Down
Loading