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

DOP-5078: Fix duplicate "docs" prefix in BreadcrumbList #1285

Merged
merged 10 commits into from
Oct 21, 2024
4 changes: 3 additions & 1 deletion src/components/Breadcrumbs/BreadcrumbContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import styled from '@emotion/styled';
import { reportAnalytics } from '../../utils/report-analytics';
import { theme } from '../../theme/docsTheme';
import { getFullBreadcrumbPath } from '../../utils/get-complete-breadcrumb-data';
import { useSiteMetadata } from '../../hooks/use-site-metadata';
import IndividualBreadcrumb from './IndividualBreadcrumb';
import CollapsedBreadcrumbs from './CollapsedBreadcrumbs';

Expand All @@ -23,6 +24,7 @@ const initialMaxCrumbs = (breadcrumbs) => breadcrumbs.length + 1;

const BreadcrumbContainer = ({ breadcrumbs }) => {
const [maxCrumbs, setMaxCrumbs] = React.useState(initialMaxCrumbs(breadcrumbs));
const { siteUrl } = useSiteMetadata();

React.useEffect(() => {
const handleResize = () => {
Expand Down Expand Up @@ -68,7 +70,7 @@ const BreadcrumbContainer = ({ breadcrumbs }) => {
setIsExcessivelyTruncated={collapseBreadcrumbs}
onClick={() =>
reportAnalytics('BreadcrumbClick', {
breadcrumbClicked: getFullBreadcrumbPath(crumb.path, true),
breadcrumbClicked: getFullBreadcrumbPath(siteUrl, crumb.path, true),
})
}
></IndividualBreadcrumb>
Expand Down
5 changes: 4 additions & 1 deletion src/components/Breadcrumbs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { theme } from '../../theme/docsTheme';
import { getCompleteBreadcrumbData } from '../../utils/get-complete-breadcrumb-data.js';
import { useBreadcrumbs } from '../../hooks/use-breadcrumbs';
import useSnootyMetadata from '../../utils/use-snooty-metadata';
import { useSiteMetadata } from '../../hooks/use-site-metadata.js';
import BreadcrumbContainer from './BreadcrumbContainer';

const breadcrumbBodyStyle = css`
Expand Down Expand Up @@ -35,17 +36,19 @@ const Breadcrumbs = ({
const { parentPaths } = useSnootyMetadata();
const parentPathsData = parentPathsProp ?? parentPaths[slug];

const { siteUrl } = useSiteMetadata();
const breadcrumbs = React.useMemo(
() =>
getCompleteBreadcrumbData({
siteUrl,
siteTitle,
slug,
queriedCrumbs,
parentPaths: parentPathsData,
selfCrumbContent: selfCrumb,
pageInfo: pageInfo,
}),
[parentPathsData, queriedCrumbs, siteTitle, slug, selfCrumb, pageInfo]
[siteUrl, parentPathsData, queriedCrumbs, siteTitle, slug, selfCrumb, pageInfo]
);

return (
Expand Down
4 changes: 0 additions & 4 deletions src/components/Footnote/FootnoteReference.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ const FootnoteReference = ({ nodeData: { id, refname } }) => {
const { footnotes } = useContext(FootnoteContext);
const { darkMode } = useDarkMode();

// the nodeData originates from docutils, and may be incorrect for
Copy link
Collaborator

Choose a reason for hiding this comment

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

guessing this change is not from this PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's one of the bonus changes mentioned above. Figured I'd just lump it in this PR since it's small and I was planning on making a commit for it anyways, but happy to make it separate if needed!

// anonymous footnoteReferences originating from included files -- docutils
// appears to assign IDs within the included files before they are collated

const ref = refname || id.replace('id', '');
const uid = refname ? `${refname}-${id}` : id;

Expand Down
38 changes: 10 additions & 28 deletions src/components/StructuredData/BreadcrumbSchema.js
Original file line number Diff line number Diff line change
@@ -1,46 +1,28 @@
import React from 'react';
import PropTypes from 'prop-types';
import { getCompleteBreadcrumbData, getFullBreadcrumbPath } from '../../utils/get-complete-breadcrumb-data.js';
import { useBreadcrumbs } from '../../hooks/use-breadcrumbs';
import useSnootyMetadata from '../../utils/use-snooty-metadata';
import { STRUCTURED_DATA_CLASSNAME } from '../../utils/structured-data.js';

const getBreadcrumbList = (breadcrumbs) =>
breadcrumbs.map(({ path, title }, index) => {
path = getFullBreadcrumbPath(path, true);

return {
'@type': 'ListItem',
position: index + 1,
name: title,
item: path,
};
});
import { BreadcrumbListSd, STRUCTURED_DATA_CLASSNAME } from '../../utils/structured-data.js';
import { useSiteMetadata } from '../../hooks/use-site-metadata.js';

const BreadcrumbSchema = ({ slug }) => {
const { parentPaths, title: siteTitle } = useSnootyMetadata();
const { siteUrl } = useSiteMetadata();

const parentPathsSlug = parentPaths[slug];

const queriedCrumbs = useBreadcrumbs();
const breadcrumbList = React.useMemo(
() => [
...getBreadcrumbList([
...getCompleteBreadcrumbData({ siteTitle, slug, queriedCrumbs, parentPaths: parentPathsSlug }),
]),
],
[siteTitle, slug, queriedCrumbs, parentPathsSlug]
);

const breadcrumbSd = React.useMemo(() => {
const sd = new BreadcrumbListSd({ siteUrl, siteTitle, slug, queriedCrumbs, parentPaths: parentPathsSlug });
return sd.isValid() ? sd.toString() : undefined;
}, [siteUrl, siteTitle, slug, queriedCrumbs, parentPathsSlug]);

return (
<>
{Array.isArray(queriedCrumbs.breadcrumbs) && (
{Array.isArray(queriedCrumbs.breadcrumbs) && breadcrumbSd && (
<script className={STRUCTURED_DATA_CLASSNAME} type="application/ld+json">
{JSON.stringify({
'@context': 'https://schema.org',
'@type': 'BreadcrumbList',
itemListElement: breadcrumbList,
})}
{breadcrumbSd}
</script>
)}
</>
Expand Down
1 change: 1 addition & 0 deletions src/hooks/use-site-metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export const useSiteMetadata = () => {
parserUser
patchId
pathPrefix
project
Copy link
Collaborator

Choose a reason for hiding this comment

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

Trying to figure out why this was added in this PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When checking for usage of siteUrl, I noticed that the build metadata object we use for this hook had project, but the hook was not returning project. I added it here for completeness so it can be used properly.

It's completely out of scope for this PR, but added it as a bonus since it was a small change. Happy to create a separate PR for it if you'd prefer!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep it in! Thank you for the explanation!

reposDatabase
siteUrl
snootyBranch
Expand Down
16 changes: 8 additions & 8 deletions src/utils/get-complete-breadcrumb-data.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { withPrefix } from 'gatsby';
import { baseUrl } from './base-url';
import { baseUrl, joinUrlAndPath } from './base-url';
import { assertTrailingSlash } from './assert-trailing-slash';
import { removeLeadingSlash } from './remove-leading-slash';
import { assertLeadingSlash } from './assert-leading-slash';
import { isRelativeUrl } from './is-relative-url';
import { getUrl, getCompleteUrl } from './url-utils';
Expand All @@ -26,17 +25,18 @@ const nodesToString = (titleNodes) => {
.join('');
};

export const getFullBreadcrumbPath = (path, needsPrefix) => {
if (needsPrefix) {
path = withPrefix(path);
}
export const getFullBreadcrumbPath = (siteUrl, path, needsPrefix) => {
seungpark marked this conversation as resolved.
Show resolved Hide resolved
if (isRelativeUrl(path)) {
path = baseUrl() + removeLeadingSlash(path);
if (needsPrefix) {
path = withPrefix(path);
}
path = joinUrlAndPath(siteUrl, path);
}
return assertTrailingSlash(path);
};

export const getCompleteBreadcrumbData = ({
siteUrl,
siteTitle,
slug,
queriedCrumbs,
Expand All @@ -50,7 +50,7 @@ export const getCompleteBreadcrumbData = ({

//get intermediate breadcrumbs
const intermediateCrumbs = (queriedCrumbs?.breadcrumbs ?? []).map((crumb) => {
return { ...crumb, path: getFullBreadcrumbPath(crumb.path, false) };
return { ...crumb, path: getFullBreadcrumbPath(siteUrl, crumb.path, false) };
});

const homeCrumb = {
Expand Down
22 changes: 22 additions & 0 deletions src/utils/structured-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import { getFullLanguageName } from './get-language';
import { findKeyValuePair } from './find-key-value-pair';
import { getPlaintext } from './get-plaintext';
import { getCompleteBreadcrumbData, getFullBreadcrumbPath } from './get-complete-breadcrumb-data';

// Class name to help Smartling identify all structured data, if needed
export const STRUCTURED_DATA_CLASSNAME = 'structured_data';
Expand Down Expand Up @@ -58,6 +59,27 @@ export class StructuredData {
}
}

export class BreadcrumbListSd extends StructuredData {
constructor({ siteUrl, siteTitle, slug, queriedCrumbs, parentPaths }) {
super('BreadcrumbList');
const breadcrumbs = getCompleteBreadcrumbData({ siteUrl, siteTitle, slug, queriedCrumbs, parentPaths });
this.itemListElement = this.getBreadcrumbList(breadcrumbs, siteUrl);
}

/**
* @param {object[]} breadcrumbs
* @param {string} siteUrl
*/
getBreadcrumbList(breadcrumbs, siteUrl) {
return breadcrumbs.map(({ path, title }, index) => ({
'@type': 'ListItem',
position: index + 1,
name: title,
item: getFullBreadcrumbPath(siteUrl, path, true),
}));
}
}

class HowToSd extends StructuredData {
constructor({ steps, name }) {
super('HowTo');
Expand Down
12 changes: 10 additions & 2 deletions tests/unit/BreadcrumbContainer.test.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,24 @@
import React from 'react';
import * as Gatsby from 'gatsby';
import { render } from '@testing-library/react';
import BreadcrumbContainer from '../../src/components/Breadcrumbs/BreadcrumbContainer';
import mockData from './data/Breadcrumbs.test.json';

jest.mock(`../../src/utils/use-snooty-metadata`, () => jest.fn());

const useStaticQuery = jest.spyOn(Gatsby, 'useStaticQuery');
useStaticQuery.mockImplementation(() => ({
site: {
siteMetadata: {
siteUrl: 'https://www.mongodb.com/',
},
},
}));

const mountBreadcrumbContainer = (breadcrumbs) => {
return render(<BreadcrumbContainer breadcrumbs={breadcrumbs} />);
};

jest.mock(`../../src/utils/use-snooty-metadata`, () => jest.fn());

const mockIntermediateCrumbs = {
title: 'MongoDB Atlas',
path: 'https://www.mongodb.com/docs/atlas/',
Expand Down
48 changes: 48 additions & 0 deletions tests/unit/BreadcrumbSchema.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import React from 'react';
import * as Gatsby from 'gatsby';
import { render } from '@testing-library/react';
import useSnootyMetadata from '../../src/utils/use-snooty-metadata';
import BreadcrumbSchema from '../../src/components/StructuredData/BreadcrumbSchema';
import { mockWithPrefix } from '../utils/mock-with-prefix';
import mockParents from './data/Breadcrumbs.test.json';

jest.mock(`../../src/utils/use-snooty-metadata`, () => jest.fn());

const mockIntermediateCrumbs = [
{
title: 'MongoDB Atlas',
path: 'https://www.mongodb.com/docs/atlas',
seungpark marked this conversation as resolved.
Show resolved Hide resolved
},
];

const useStaticQuery = jest.spyOn(Gatsby, 'useStaticQuery');
useStaticQuery.mockImplementation(() => ({
site: {
siteMetadata: {
siteUrl: 'https://www.mongodb.com/',
},
},
allBreadcrumb: {
nodes: [
{
project: 'realm',
breadcrumbs: mockIntermediateCrumbs,
propertyUrl: 'https://www.mongodb.com/docs/atlas/device-sdks/',
},
],
},
}));

describe('BreadcrumbSchema', () => {
beforeAll(() => {
useSnootyMetadata.mockImplementation(() => ({
parentPaths: mockParents,
}));
mockWithPrefix('/docs/atlas/device-sdks');
});

it('returns correct structured data with parents and intermediate breadcrumbs', () => {
const { asFragment } = render(<BreadcrumbSchema slug={'sdk/cpp/app-services/call-a-function'} />);
expect(asFragment()).toMatchSnapshot();
});
});
10 changes: 9 additions & 1 deletion tests/unit/Breadcrumbs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { render } from '@testing-library/react';
import Breadcrumbs from '../../src/components/Breadcrumbs/index';
import useSnootyMetadata from '../../src/utils/use-snooty-metadata';

import { mockWithPrefix } from '../utils/mock-with-prefix';
import mockData from './data/Breadcrumbs.test.json';

jest.mock(`../../src/utils/use-snooty-metadata`, () => jest.fn());
Expand All @@ -18,7 +19,7 @@ beforeAll(() => {
const mockIntermediateCrumbs = [
{
title: 'MongoDB Atlas',
path: '/atlas',
path: 'https://www.mongodb.com/docs/atlas/',
},
];
const useStaticQuery = jest.spyOn(Gatsby, 'useStaticQuery');
Expand All @@ -34,8 +35,15 @@ useStaticQuery.mockImplementation(() => ({
},
],
},
site: {
siteMetadata: {
siteUrl: 'https://www.mongodb.com/',
},
},
}));

mockWithPrefix('/docs/atlas/device-sdks');

it('renders correctly with siteTitle', () => {
const tree = render(<Breadcrumbs siteTitle={'Atlas Device SDKs'} slug={'sdk/cpp/app-services/call-a-function'} />);
expect(tree.asFragment()).toMatchSnapshot();
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/__snapshots__/BreadcrumbSchema.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`BreadcrumbSchema returns correct structured data with parents and intermediate breadcrumbs 1`] = `
<DocumentFragment>
<script
class="structured_data"
type="application/ld+json"
>
{"@context":"https://schema.org","@type":"BreadcrumbList","itemListElement":[{"@type":"ListItem","position":1,"name":"Docs Home","item":"https://www.mongodb.com/docs/"},{"@type":"ListItem","position":2,"name":"MongoDB Atlas","item":"https://www.mongodb.com/docs/atlas/"},{"@type":"ListItem","position":3,"item":"https://www.mongodb.com/docs/atlas/device-sdks/"},{"@type":"ListItem","position":4,"name":"Atlas Device SDK for C++","item":"https://www.mongodb.com/docs/atlas/device-sdks/sdk/cpp/"},{"@type":"ListItem","position":5,"name":"Application Services - C++ SDK","item":"https://www.mongodb.com/docs/atlas/device-sdks/sdk/cpp/application-services/"}]}
</script>
</DocumentFragment>
`;
24 changes: 24 additions & 0 deletions tests/utils/mock-with-prefix.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import * as Gatsby from 'gatsby';

const withPrefix = jest.spyOn(Gatsby, 'withPrefix');

export const mockWithPrefix = (prefix) => {
withPrefix.mockImplementation((path) => {
let normalizedPrefix = prefix;
let normalizedPath = path;

if (!normalizedPrefix.startsWith('/')) {
normalizedPrefix = `/${normalizedPrefix}`;
}

if (normalizedPrefix.endsWith('/')) {
normalizedPrefix = normalizedPath.slice(0, -1);
}

if (normalizedPath.startsWith('/')) {
normalizedPath = normalizedPath.slice(1);
}

return `${normalizedPrefix}/${normalizedPath}`;
});
};
Loading