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

Add external doc sidebar links and replace "Changelog" with one. #1273

Merged
merged 15 commits into from
May 9, 2020
Merged
Changes from all commits
Commits
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
48 changes: 0 additions & 48 deletions content/docs/changelog/0.18.md

This file was deleted.

79 changes: 0 additions & 79 deletions content/docs/changelog/0.35.md

This file was deleted.

15 changes: 3 additions & 12 deletions content/docs/sidebar.json
Original file line number Diff line number Diff line change
@@ -440,17 +440,8 @@
]
},
{
"slug": "changelog",
"source": false,
"children": [
{
"label": "v0.12 - v0.18",
"slug": "0.18"
},
{
"label": "v0.19 - v0.35",
"slug": "0.35"
}
]
"label": "Changelog",
"url": "https://github.com/iterative/dvc/releases",
"type": "external"
}
]
2 changes: 1 addition & 1 deletion src/components/Community/data.json
Original file line number Diff line number Diff line change
@@ -38,7 +38,7 @@
"description": "Step-by-step introduction to basic DVC features"
},
{
"url": "/doc/changelog",
"url": "https://github.com/iterative/dvc/releases",
"title": "Changelog",
"description": "See what's new in DVC."
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
38 changes: 29 additions & 9 deletions src/components/Documentation/Layout/SidebarMenu/index.tsx
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ import includes from 'lodash/includes'
import ShowOnly from '../../../ShowOnly'
import DownloadButton from '../../../DownloadButton'
import Link from '../../../Link'
import { ReactComponent as ExternalLinkIcon } from './external-link-icon.svg'

import {
structure,
@@ -25,34 +26,53 @@ interface ISidebarMenuItemProps {
source: boolean | string
onClick: (e: React.MouseEvent) => void
activePaths?: Array<string>
type?: string
}

const SidebarMenuItem: React.FC<ISidebarMenuItemProps> = ({
children,
label,
path,
activePaths,
onClick
onClick,
type
}) => {
const isActive = activePaths && includes(activePaths, path)
const isRootParent =
activePaths && activePaths.length > 1 && activePaths[0] === path

return (
<>
const className = cn(
styles.sectionLink,
isActive && styles.active,
isRootParent && 'docSearch-lvl0',
'link-with-focus'
)

const parentElement =
type === 'external' ? (
<Link
href={path}
id={path}
className={className}
onClick={onClick}
target="_blank"
>
{label} <ExternalLinkIcon />
</Link>
) : (
<Link
href={getPathWithSource(path)}
id={path}
className={cn(
styles.sectionLink,
isActive && styles.active,
isRootParent && 'docSearch-lvl0',
'link-with-focus'
)}
className={className}
onClick={onClick}
>
{label}
</Link>
)

return (
<>
{parentElement}
{children && (
<Collapse isOpened={!!isActive}>
{children.map(item => (
15 changes: 11 additions & 4 deletions src/components/Link/index.tsx
Original file line number Diff line number Diff line change
@@ -21,12 +21,13 @@ const ResultLinkComponent: React.FC<ILinkProps> = ({
href,
children,
rel,
target,
...restProps
}) => {
// Handle all situations where a basic `a` must be used over Gatsby Link
const hrefIsRelative = isRelative(href)
const hrefIsMailto = isMailto(href)
const hrefHasTarget = restProps.target
const hrefHasTarget = typeof target === 'string'
// Fragments within the page should be `a`, but links to other pages
// that have anchors should be okay.
const hrefIsRelativeFragment = href.startsWith('#')
@@ -40,13 +41,19 @@ const ResultLinkComponent: React.FC<ILinkProps> = ({
/*
Change external links without an explicit rel to have 'noopener
noreferrer', but leave explicitly defined rels alone.
Do the same with `target=_blank`
*/
if (!hrefIsRelative && typeof rel !== 'string') {
rel = 'noopener noreferrer'
if (!hrefIsRelative) {
if (typeof rel !== 'string') {
rel = 'noopener noreferrer'
}
if (!hrefHasTarget) {
target = '_blank'
}
}

return (
<a href={href} rel={rel} {...restProps}>
<a href={href} rel={rel} target={target} {...restProps}>
{children}
</a>
)
69 changes: 45 additions & 24 deletions src/utils/shared/sidebar.js
Original file line number Diff line number Diff line change
@@ -25,17 +25,25 @@ const PATH_ROOT = '/doc'
const FILE_ROOT = '/docs/'
const FILE_EXTENSION = '.md'

function validateRawItem({ slug, source, children }) {
function validateRawItem({ slug, source, children, type, url }) {
const isSourceDisabled = source === false

if (typeof slug !== 'string') {
throw Error("'slug' field is required in objects in sidebar.json")
}
switch (type) {
case 'external':
if (typeof url !== 'string') {
throw Error("'url' field is required in external sidebar.json entries")
}
break
default:
if (typeof slug !== 'string') {
throw Error("'slug' field is required in local sidebar.json entries")
}

if (isSourceDisabled && (!children || !children.length)) {
throw Error(
"If you set 'source' to false, you had to add at least one child"
)
if (isSourceDisabled && (!children || !children.length)) {
throw Error(
'Local sidebar.json entries with no source must have children'
)
}
}
}

@@ -69,26 +77,35 @@ function findPrevItemWithSource(data, item) {
function normalizeItem({ rawItem, parentPath, resultRef, prevRef }) {
validateRawItem(rawItem)

const { label, slug, source, tutorials } = rawItem
const { label, slug, source, tutorials, type, url } = rawItem

// If prev item doesn't have source we need to search for it
const prevItemWithSource =
prevRef && findPrevItemWithSource(resultRef, prevRef)
switch (type) {
case 'external':
return {
type,
path: url,
label
}
default:
// If prev item doesn't have source we need to search for it
const prevItemWithSource =
prevRef && findPrevItemWithSource(resultRef, prevRef)

const prev = prevItemWithSource && prevItemWithSource.path
const prev = prevItemWithSource && prevItemWithSource.path

const sourceFileName = source ? source : slug + FILE_EXTENSION
const sourcePath = FILE_ROOT + parentPath + sourceFileName
const sourceFileName = source ? source : slug + FILE_EXTENSION
const sourcePath = FILE_ROOT + parentPath + sourceFileName

const relativePath = parentPath + slug
const relativePath = parentPath + slug

return {
path: relativePath ? `${PATH_ROOT}/${relativePath}` : PATH_ROOT,
source: source === false ? false : sourcePath,
label: label ? label : startCase(slug),
tutorials: tutorials || {},
prev,
next: undefined
return {
path: relativePath ? `${PATH_ROOT}/${relativePath}` : PATH_ROOT,
source: source === false ? false : sourcePath,
label: label ? label : startCase(slug),
tutorials: tutorials || {},
prev,
next: undefined
}
}
}

@@ -145,7 +162,11 @@ const normalizedSidebar = normalizeSidebar({
})

function findChildWithSource(item) {
return item.source ? item : findChildWithSource(item.children[0])
// Return item unchanged if isn't root-relative
if (!item.path.startsWith('/')) return item
return item.source
? item
: findChildWithSource(item.children && item.children[0])
}

function getFirstPage() {
16 changes: 13 additions & 3 deletions src/utils/shared/sidebar.test.js
Original file line number Diff line number Diff line change
@@ -292,13 +292,23 @@ describe('SidebarMenu/helper', () => {
expect(sidebarData).toEqual(result)
})

it("Throws error if item didn't have slug field", () => {
it("Throws error if external item doesn't have a url field", () => {
const rawData = [{ type: 'external' }]

jest.doMock('../../../content/docs/sidebar.json', () => rawData)

expect(() => require('./sidebar')).toThrow(
new Error("'url' field is required in external sidebar.json entries")
)
})

it("Throws error if local item doesn't have slug field", () => {
const rawData = [{}]

jest.doMock('../../../content/docs/sidebar.json', () => rawData)

expect(() => require('./sidebar')).toThrow(
new Error("'slug' field is required in objects in sidebar.json")
new Error("'slug' field is required in local sidebar.json entries")
)
})

@@ -310,7 +320,7 @@ describe('SidebarMenu/helper', () => {

expect(() => require('./sidebar')).toThrow(
new Error(
"If you set 'source' to false, you had to add at least one child"
'Local sidebar.json entries with no source must have children'
)
)
})