-
Notifications
You must be signed in to change notification settings - Fork 394
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 doc/user-guide/glossary
page
#2595
Changes from 7 commits
511f66d
ca9c4fa
4b42509
6bdea40
1b20e14
fda5dea
7edac93
f17b601
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -100,7 +100,7 @@ | |||||||||||||
}, | ||||||||||||||
{ | ||||||||||||||
"slug": "project-structure", | ||||||||||||||
"source": "user-guide/project-structure/index.md", | ||||||||||||||
"source": "project-structure/index.md", | ||||||||||||||
"children": [ | ||||||||||||||
{ | ||||||||||||||
"label": "Pipelines Files (dvc.yaml)", | ||||||||||||||
|
@@ -138,7 +138,7 @@ | |||||||||||||
{ | ||||||||||||||
"label": "Experiment Management", | ||||||||||||||
"slug": "experiment-management", | ||||||||||||||
"source": "user-guide/experiment-management/index.md", | ||||||||||||||
"source": "experiment-management/index.md", | ||||||||||||||
"children": ["checkpoints"] | ||||||||||||||
}, | ||||||||||||||
"setup-google-drive-remote", | ||||||||||||||
|
@@ -173,6 +173,11 @@ | |||||||||||||
{ | ||||||||||||||
"label": "Anonymized Usage Analytics", | ||||||||||||||
"slug": "analytics" | ||||||||||||||
}, | ||||||||||||||
{ | ||||||||||||||
"label": "Glossary", | ||||||||||||||
"slug": "glossary", | ||||||||||||||
"source": "basic-concepts" | ||||||||||||||
} | ||||||||||||||
Comment on lines
+177
to
181
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would it take to reduce this sidebar entry to just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if I'm understanding your question 🤔 Are you referring to deleting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think by "sidebar entry" you meant page URL slug? The sidebar entry is "Glossary," and the page URL slug is "glossary." The "source" folder is only used for the "Edit on GH" button. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I was referring to this:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Right. Should we do that? But again... #550 (see #2595 (review) below). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we'd be able to do that(replacing the whole object with "glossary") since the "Edit on Github" link would default to https://github.com/iterative/dvc.org/blob/master/content/docs/user-guide/glossary.md which doesn't exist. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just noticed this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait no I see. I think it could default to /content/docs/user-guide/glossary/index.md too? Again, this is merged so np but I was thinking what does it take to decouple it completely from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. confusion related to #2595 (comment) |
||||||||||||||
] | ||||||||||||||
}, | ||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
import React, { useEffect, useRef, useCallback } from 'react' | ||
import cn from 'classnames' | ||
import { navigate } from '@reach/router' | ||
|
||
import Link from '../../../Link' | ||
import Tutorials from '../../TutorialsLinks' | ||
import { getPathWithSource } from '../../../../utils/shared/sidebar' | ||
|
||
import 'github-markdown-css/github-markdown.css' | ||
import sharedStyles from '../../styles.module.css' | ||
import styles from './styles.module.css' | ||
|
||
const isInsideCodeBlock = (node: Element): boolean => { | ||
while (node?.parentNode) { | ||
if (node.tagName === 'PRE') { | ||
return true | ||
} | ||
|
||
if (node.tagName === 'ARTICLE') { | ||
return false | ||
} | ||
|
||
node = node.parentNode as Element | ||
} | ||
|
||
return false | ||
} | ||
|
||
interface IMainProps { | ||
githubLink: string | ||
tutorials: { [type: string]: string } | ||
prev?: string | ||
next?: string | ||
} | ||
|
||
const Main: React.FC<IMainProps> = ({ | ||
children, | ||
prev, | ||
next, | ||
tutorials, | ||
githubLink | ||
}) => { | ||
const touchstartXRef = useRef(0) | ||
const touchendXRef = useRef(0) | ||
const isCodeBlockRef = useRef(false) | ||
const handleSwipeGesture = useCallback(() => { | ||
if (isCodeBlockRef.current) return | ||
|
||
if (next && touchstartXRef.current - touchendXRef.current > 100) { | ||
navigate(next) | ||
} | ||
|
||
if (prev && touchendXRef.current - touchstartXRef.current > 100) { | ||
navigate(prev) | ||
} | ||
}, [prev, next]) | ||
const onTouchStart = useCallback(e => { | ||
isCodeBlockRef.current = isInsideCodeBlock(e.target) | ||
touchstartXRef.current = e.changedTouches[0].screenX | ||
}, []) | ||
const onTouchEnd = useCallback(e => { | ||
touchendXRef.current = e.changedTouches[0].screenX | ||
handleSwipeGesture() | ||
}, []) | ||
|
||
useEffect(() => { | ||
document.addEventListener('touchstart', onTouchStart, false) | ||
document.addEventListener('touchend', onTouchEnd, false) | ||
|
||
return (): void => { | ||
document.removeEventListener('touchstart', onTouchStart) | ||
document.removeEventListener('touchend', onTouchEnd) | ||
} | ||
}, []) | ||
|
||
return ( | ||
<div className={styles.content} id="markdown-root"> | ||
{tutorials && ( | ||
<div className={styles.tutorialsWrapper}> | ||
<Tutorials tutorials={tutorials} compact={true} /> | ||
</div> | ||
)} | ||
<Link | ||
className={cn(sharedStyles.button, styles.githubLink)} | ||
href={githubLink} | ||
target="_blank" | ||
> | ||
<i className={cn(sharedStyles.buttonIcon, styles.githubIcon)} /> Edit on | ||
GitHub | ||
</Link> | ||
<div className="markdown-body">{children}</div> | ||
<div className={styles.navButtons}> | ||
<Link className={styles.navButton} href={prev || '#'}> | ||
<i className={cn(styles.navButtonIcon, styles.prev)} /> | ||
<span>Prev</span> | ||
</Link> | ||
<Link | ||
className={styles.navButton} | ||
href={next ? getPathWithSource(next) : '#'} | ||
> | ||
<span>Next</span> | ||
<i className={cn(styles.navButtonIcon, styles.next)} /> | ||
</Link> | ||
</div> | ||
</div> | ||
) | ||
} | ||
|
||
export default Main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that this page and the page on line
143
had incorrectsource
causing the "Edit on Github" link to take you to a 404 page. I went ahead and fixed them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Should the engine throw an error when building? Out of scope for this PR but could open an issue (or move the fix to another PR and we can decide there). Same for line 141
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about the engine throwing an error, but I don't mind moving these fixes to a separate PR if needed!