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

Preview markdown files #4153

Merged
merged 32 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
31c4062
preview markdown files
fiskus Sep 24, 2024
2fefa50
visibility icon
fiskus Sep 24, 2024
82ad642
Merge branch 'master' of github.com:quiltdata/quilt into fileeditor-p…
fiskus Sep 27, 2024
e0c72a8
add one level of abstraction for previewing
fiskus Sep 27, 2024
f194c08
Merge branch 'master' of github.com:quiltdata/quilt into fileeditor-p…
fiskus Oct 4, 2024
67426fb
minor polishing
fiskus Oct 4, 2024
56fac53
TODO notes
fiskus Oct 4, 2024
79efd76
merge with master
fiskus Oct 10, 2024
c676860
Merge branch 'master' of github.com:quiltdata/quilt into fileeditor-p…
fiskus Nov 15, 2024
ff5c00d
simplify UI by using M.Switch
fiskus Nov 15, 2024
5ca7019
stoppropagation
fiskus Nov 15, 2024
7642f02
lazy load markdown text renderer
fiskus Nov 19, 2024
474259a
isolate getting if preview available to some extent
fiskus Nov 19, 2024
066d161
use type union
fiskus Nov 20, 2024
8294326
move to Preview/quick
fiskus Nov 20, 2024
59a49ae
add test for quick/index
fiskus Nov 20, 2024
a7119b4
basic render tests
fiskus Nov 20, 2024
1d5db13
Merge branch 'master' of github.com:quiltdata/quilt into fileeditor-p…
fiskus Nov 20, 2024
6c0cd1d
add changelog entry
fiskus Nov 20, 2024
43cd62d
Merge branch 'master' of github.com:quiltdata/quilt into fileeditor-p…
fiskus Nov 21, 2024
440a56e
Update catalog/CHANGELOG.md
fiskus Nov 21, 2024
1e6baeb
remove leftover function and update the lint ruleset to catch this ne…
fiskus Nov 21, 2024
8459ef5
memoize asyncresult
fiskus Nov 21, 2024
6f41bdf
remove Pending handler
fiskus Nov 21, 2024
e450309
make lines a distinct constant
fiskus Nov 21, 2024
930d367
Merge branch 'fileeditor-preview' of github.com:quiltdata/quilt into …
fiskus Nov 21, 2024
aea70d2
remove obsolete test
fiskus Nov 21, 2024
ade1b62
handle all states explicitly by showing error on "unhandled" states
fiskus Nov 21, 2024
7a6ae2f
Merge branch 'master' of github.com:quiltdata/quilt into fileeditor-p…
fiskus Nov 21, 2024
2d9d421
merge with master
fiskus Nov 21, 2024
b00b936
resolve changelog conflict
fiskus Nov 25, 2024
b79d128
resolve changelog conflict
fiskus Nov 25, 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
1 change: 1 addition & 0 deletions catalog/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ module.exports = {
'max-classes-per-file': 0,
'no-console': 2,
'no-nested-ternary': 1,
'no-restricted-globals': [2, "event", "location", "stop"],
'no-underscore-dangle': [2, { allow: ['_', '__', '__typename', '_tag'] }],
'prefer-arrow-callback': [2, { allowNamedFunctions: true }],
'prefer-template': 2,
Expand Down
1 change: 1 addition & 0 deletions catalog/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ where verb is one of

## Changes

- [Added] Preview Markdown while editing ([#4153](https://github.com/quiltdata/quilt/pull/4153))
- [Changed] Athena: hide data catalogs user doesn't have access to ([#4239](https://github.com/quiltdata/quilt/pull/4239))
- [Added] Enable MixPanel tracking in Embed mode ([#4237](https://github.com/quiltdata/quilt/pull/4237))
- [Fixed] Fix embed files listing ([#4236](https://github.com/quiltdata/quilt/pull/4236))
Expand Down
20 changes: 20 additions & 0 deletions catalog/app/components/FileEditor/Controls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,26 @@
)
}

interface PreviewButtonProps extends EditorState {
className?: string
onPreview: NonNullable<EditorState['onPreview']>
}

export function PreviewButton({ className, preview, onPreview }: PreviewButtonProps) {
const handleClick = React.useCallback(() => onPreview(!preview), [onPreview, preview])
return (

Check warning on line 26 in catalog/app/components/FileEditor/Controls.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/FileEditor/Controls.tsx#L24-L26

Added lines #L24 - L26 were not covered by tests
<M.FormControlLabel
onClick={(event) => event.stopPropagation()}

Check warning on line 28 in catalog/app/components/FileEditor/Controls.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/FileEditor/Controls.tsx#L28

Added line #L28 was not covered by tests
className={className}
control={
<M.Switch checked={preview} onChange={handleClick} size="small" color="primary" />
}
label="Preview"
labelPlacement="end"
/>
)
}

interface ControlsProps extends EditorState {
className?: string
}
Expand Down
25 changes: 23 additions & 2 deletions catalog/app/components/FileEditor/FileEditor.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import cx from 'classnames'

Check warning on line 1 in catalog/app/components/FileEditor/FileEditor.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/FileEditor/FileEditor.tsx#L1

Added line #L1 was not covered by tests
import * as React from 'react'
import * as M from '@material-ui/core'

Check warning on line 3 in catalog/app/components/FileEditor/FileEditor.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/FileEditor/FileEditor.tsx#L3

Added line #L3 was not covered by tests

import * as PreviewUtils from 'components/Preview/loaders/utils'
import PreviewDisplay from 'components/Preview/Display'
import * as PreviewUtils from 'components/Preview/loaders/utils'
import { QuickPreview } from 'components/Preview/quick'

Check warning on line 7 in catalog/app/components/FileEditor/FileEditor.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/FileEditor/FileEditor.tsx#L6-L7

Added lines #L6 - L7 were not covered by tests
import type * as Model from 'model'
import AsyncResult from 'utils/AsyncResult'

Expand Down Expand Up @@ -95,10 +98,28 @@
})
}

const useStyles = M.makeStyles({

Check warning on line 101 in catalog/app/components/FileEditor/FileEditor.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/FileEditor/FileEditor.tsx#L101

Added line #L101 was not covered by tests
tab: {
display: 'none',
width: '100%',
},
active: {
display: 'block',
},
})

export function Editor(props: EditorProps) {
const classes = useStyles()

Check warning on line 112 in catalog/app/components/FileEditor/FileEditor.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/FileEditor/FileEditor.tsx#L112

Added line #L112 was not covered by tests
return (
<React.Suspense fallback={<Skeleton />}>
<EditorSuspended {...props} />
<div className={cx(classes.tab, { [classes.active]: !props.preview })}>
<EditorSuspended {...props} />
</div>
{props.preview && (
<div className={cx(classes.tab, classes.active)}>

Check warning on line 119 in catalog/app/components/FileEditor/FileEditor.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/FileEditor/FileEditor.tsx#L119

Added line #L119 was not covered by tests
<QuickPreview handle={props.handle} type={props.editing} value={props.value} />
</div>
)}
</React.Suspense>
)
}
8 changes: 7 additions & 1 deletion catalog/app/components/FileEditor/State.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import * as RRDom from 'react-router-dom'

import type * as Model from 'model'
import { isQuickPreviewAvailable } from 'components/Preview/quick'

Check warning on line 5 in catalog/app/components/FileEditor/State.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/FileEditor/State.tsx#L5

Added line #L5 was not covered by tests
import * as AddToPackage from 'containers/AddToPackage'
import * as NamedRoutes from 'utils/NamedRoutes'
import parseSearch from 'utils/parseSearch'
Expand Down Expand Up @@ -32,7 +33,9 @@
onCancel: () => void
onChange: (value: string) => void
onEdit: (type: EditorInputType | null) => void
onPreview: ((p: boolean) => void) | null
onSave: () => Promise<Model.S3File | void>
preview: boolean
saving: boolean
types: EditorInputType[]
value?: string
Expand All @@ -48,6 +51,7 @@
const [editing, setEditing] = React.useState<EditorInputType | null>(
edit ? types[0] : null,
)
const [preview, setPreview] = React.useState<boolean>(false)

Check warning on line 54 in catalog/app/components/FileEditor/State.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/FileEditor/State.tsx#L54

Added line #L54 was not covered by tests
const [saving, setSaving] = React.useState<boolean>(false)
const writeFile = useWriteData(handle)
const redirect = useRedirect()
Expand Down Expand Up @@ -80,11 +84,13 @@
onCancel,
onChange: setValue,
onEdit: setEditing,
onPreview: isQuickPreviewAvailable(editing) ? setPreview : null,
onSave,
preview,
saving,
types,
value,
}),
[editing, error, onCancel, onSave, saving, types, value],
[editing, error, onCancel, onSave, preview, saving, types, value],
)
}
1 change: 1 addition & 0 deletions catalog/app/components/FileEditor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
export * from './CreateFile'
export * from './FileEditor'
export * from './State'
export * from './types'

Check warning on line 5 in catalog/app/components/FileEditor/index.ts

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/FileEditor/index.ts#L5

Added line #L5 was not covered by tests
47 changes: 30 additions & 17 deletions catalog/app/components/Preview/loaders/Markdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import AsyncResult from 'utils/AsyncResult'
import * as NamedRoutes from 'utils/NamedRoutes'
import * as Resource from 'utils/Resource'
import pipeThru from 'utils/pipeThru'
import { resolveKey } from 'utils/s3paths'
import useMemoEq from 'utils/useMemoEq'

Expand Down Expand Up @@ -66,20 +65,36 @@
)
}

// result: AsyncResult<{Ok: {contents: string}>,
// handle: Model.S3ObjectLocation
export function useMarkdownRenderer(contentsResult, handle) {
const processImg = useImgProcessor(handle)
const processLink = useLinkProcessor(handle)
return utils.useProcessing(contentsResult, getRenderer({ processImg, processLink }), [

Check warning on line 73 in catalog/app/components/Preview/loaders/Markdown.js

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/Preview/loaders/Markdown.js#L71-L73

Added lines #L71 - L73 were not covered by tests
processImg,
processLink,
])
}

export const detect = utils.extIn(['.md', '.rmd'])

function MarkdownLoader({ gated, handle, children }) {
const processImg = useImgProcessor(handle)
const processLink = useLinkProcessor(handle)
const data = utils.useObjectGetter(handle, { noAutoFetch: gated })
const processed = utils.useProcessing(
data.result,
(r) => {
const contents = r.Body.toString('utf-8')
const rendered = getRenderer({ processImg, processLink })(contents)
return PreviewData.Markdown({ rendered, modes: [FileType.Markdown, FileType.Text] })
},
[processImg, processLink],
const contents = React.useMemo(
() =>
AsyncResult.mapCase({
Ok: (r) => r.Body.toString('utf-8'),

Check warning on line 86 in catalog/app/components/Preview/loaders/Markdown.js

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/Preview/loaders/Markdown.js#L83-L86

Added lines #L83 - L86 were not covered by tests
})(data.result),
[data.result],
)
const markdowned = useMarkdownRenderer(contents, handle)
const processed = React.useMemo(
() =>
AsyncResult.mapCase({
Ok: (rendered) =>
PreviewData.Markdown({ rendered, modes: [FileType.Markdown, FileType.Text] }),

Check warning on line 95 in catalog/app/components/Preview/loaders/Markdown.js

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/Preview/loaders/Markdown.js#L90-L95

Added lines #L90 - L95 were not covered by tests
})(markdowned),
[markdowned],
)
const handled = utils.useErrorHandling(processed, { handle, retry: data.fetch })
const result =
Expand All @@ -96,10 +111,8 @@
export const Loader = function GatedMarkdownLoader({ handle, children }) {
const data = useGate(handle, SIZE_THRESHOLDS)
const handled = utils.useErrorHandling(data.result, { handle, retry: data.fetch })
return pipeThru(handled)(
AsyncResult.case({
_: children,
Ok: (gated) => <MarkdownLoader {...{ gated, handle, children }} />,
}),
)
return AsyncResult.case({

Check warning on line 114 in catalog/app/components/Preview/loaders/Markdown.js

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/Preview/loaders/Markdown.js#L114

Added line #L114 was not covered by tests
_: children,
Ok: (gated) => <MarkdownLoader {...{ gated, handle, children }} />,

Check warning on line 116 in catalog/app/components/Preview/loaders/Markdown.js

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/Preview/loaders/Markdown.js#L116

Added line #L116 was not covered by tests
})(handled)
}
64 changes: 64 additions & 0 deletions catalog/app/components/Preview/quick/Markdown/Render.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import * as React from 'react'
import renderer from 'react-test-renderer'

import AsyncResult from 'utils/AsyncResult'

import Render from './Render'

jest.mock(
'constants/config',
jest.fn(() => {}),
)

const useMarkdownRenderer = jest.fn()
jest.mock('components/Preview/loaders/Markdown', () => ({
...jest.requireActual('components/Preview/loaders/Markdown'),
useMarkdownRenderer: jest.fn(() => useMarkdownRenderer()),
}))

jest.mock(
'components/Preview/renderers/Markdown',
() =>
({ rendered }: { rendered: string }) => (
// eslint-disable-next-line react/no-danger
<b dangerouslySetInnerHTML={{ __html: rendered }}>Markdown</b>
),
)

jest.mock(
'@material-ui/lab',
jest.fn(() => ({
Alert: ({ children }: { children: string }) => <div>Error: {children}</div>,
})),
)

const handle = {
bucket: 'foo',
key: 'bar',
}

describe('app/components/Preview/quick/Render.spec.tsx', () => {
it('it shows the error for Init state, because it is intended to run with already resolved value', () => {
useMarkdownRenderer.mockReturnValue(AsyncResult.Init())
const tree = renderer.create(<Render {...{ handle, value: 'any' }} />).toJSON()
expect(tree).toMatchSnapshot()
})

it('it shows the error for Pending state, because it is intended to run with already resolved value', () => {
useMarkdownRenderer.mockReturnValue(AsyncResult.Pending())
const tree = renderer.create(<Render {...{ handle, value: 'any' }} />).toJSON()
expect(tree).toMatchSnapshot()
})

it('returns error on Err', () => {
useMarkdownRenderer.mockReturnValue(AsyncResult.Err(new Error('some error')))
const tree = renderer.create(<Render {...{ handle, value: 'any' }} />).toJSON()
expect(tree).toMatchSnapshot()
})

it('returns markdown on data', () => {
useMarkdownRenderer.mockReturnValue(AsyncResult.Ok('<h1>It works</h1>'))
const tree = renderer.create(<Render {...{ handle, value: 'any' }} />).toJSON()
expect(tree).toMatchSnapshot()
})
})
30 changes: 30 additions & 0 deletions catalog/app/components/Preview/quick/Markdown/Render.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import * as React from 'react'
import * as Lab from '@material-ui/lab'

import { useMarkdownRenderer } from 'components/Preview/loaders/Markdown'
import Markdown from 'components/Preview/renderers/Markdown'
import type * as Model from 'model'
import AsyncResult from 'utils/AsyncResult'

import Skeleton from './Skeleton'

interface RenderProps {
value: string
handle: Model.S3.S3ObjectLocation
}

export default function Render({ value, handle }: RenderProps) {
const result = useMarkdownRenderer(AsyncResult.Ok(value), handle)
// `result` is never `Pending` or `Init`, because we pass the `AsyncResult.Ok` value
// but it can be `Err` if some post-processing fails
return AsyncResult.case({
_: (x: { value?: Error }) => (
<Lab.Alert severity="error">{x.value?.message || 'Unexpected state'}</Lab.Alert>
),
Ok: (rendered: string) => (
<React.Suspense fallback={<Skeleton />}>
<Markdown rendered={rendered} />
</React.Suspense>
),
})(result)
}
24 changes: 24 additions & 0 deletions catalog/app/components/Preview/quick/Markdown/Skeleton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import * as React from 'react'
import * as M from '@material-ui/core'

import Skel from 'components/Skeleton'

const useSkeletonStyles = M.makeStyles((t) => ({
line: {
height: t.spacing(3),
marginBottom: t.spacing(1),
},
}))

const LINES = [80, 50, 100, 60, 30, 80, 50, 100, 60, 30, 20, 70]

export default function Skeleton() {
const classes = useSkeletonStyles()
return (

Check warning on line 17 in catalog/app/components/Preview/quick/Markdown/Skeleton.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/Preview/quick/Markdown/Skeleton.tsx#L16-L17

Added lines #L16 - L17 were not covered by tests
<div>
{LINES.map((width, index) => (

Check warning on line 19 in catalog/app/components/Preview/quick/Markdown/Skeleton.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/Preview/quick/Markdown/Skeleton.tsx#L19

Added line #L19 was not covered by tests
<Skel className={classes.line} width={`${width}%`} key={width + index} />
))}
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`app/components/Preview/quick/Render.spec.tsx it shows the error for Init state, because it is intended to run with already resolved value 1`] = `
<div>
Error:
Unexpected state
</div>
`;

exports[`app/components/Preview/quick/Render.spec.tsx it shows the error for Pending state, because it is intended to run with already resolved value 1`] = `
<div>
Error:
Unexpected state
</div>
`;

exports[`app/components/Preview/quick/Render.spec.tsx returns error on Err 1`] = `
<div>
Error:
some error
</div>
`;

exports[`app/components/Preview/quick/Render.spec.tsx returns markdown on data 1`] = `
<b
dangerouslySetInnerHTML={
{
"__html": "<h1>It works</h1>",
}
}
>
Markdown
</b>
`;
15 changes: 15 additions & 0 deletions catalog/app/components/Preview/quick/Markdown/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import * as React from 'react'

import Skeleton from './Skeleton'

const RenderLazy = React.lazy(() => import('./Render'))

export { default as Skeleton } from './Skeleton'

export function Render(props: Parameters<typeof RenderLazy>[0]) {
return (

Check warning on line 10 in catalog/app/components/Preview/quick/Markdown/index.tsx

View check run for this annotation

Codecov / codecov/patch/informational

catalog/app/components/Preview/quick/Markdown/index.tsx#L10

Added line #L10 was not covered by tests
<React.Suspense fallback={<Skeleton />}>
<RenderLazy {...props} />
</React.Suspense>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`app/components/Preview/quick/index.spec.tsx QuickPreview renders Markdown 1`] = `
<h1>
This is Markdown quick preview
</h1>
`;

exports[`app/components/Preview/quick/index.spec.tsx QuickPreview renders no preview if unsupported file type 1`] = `
<p
className="MuiTypography-root MuiTypography-body1"
>
There is no content for quick preview
</p>
`;

exports[`app/components/Preview/quick/index.spec.tsx QuickPreview renders no value 1`] = `
<p
className="MuiTypography-root MuiTypography-body1"
>
There is no content for quick preview
</p>
`;
Loading