ByoungSooPark/diff-viewer-extension
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add toggle between source and rich diff for CAD (#36)

Browse Source 
* Remove await on GetFileDiff
Progress towards #28

* n roots instead of 2n

* Lint

* Trying something with portals

* Portal component, one root

* Fix element clean up, prep for later tasks

* Draft: toolbar element injection

* Working click to select rich or source

* Actually working now

* Fixes

* Clean up

* Polishing here and there

* Add SourceRichToggle component

* e2e test with toolbar

* Update linux snapshots

* Remove failing test (WIP)

* Clean up
This commit is contained in:
Pierre Jacquier
2023-03-27 16:37:21 -04:00
committed by GitHub
parent 0be1f4ef30
commit 3df22f1116
13 changed files with 203 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
.pnp.cjs generated
View File

@ -34,6 +34,7 @@ function $$SETUP_STATE(hydrateRuntimeState, basePath) {
["@octokit/rest", "npm:19.0.7"],\
["@octokit/types", "npm:9.0.0"],\
["@playwright/test", "npm:1.31.2"],\
["@primer/octicons-react", "virtual:ff5ad3439f8ec237c0c86796b437c422a681ce7f1211bc52c84c467fd5c19025673469e4b3bd047db74cf4144d670522e4013a081fcd63cc449dbcb3b5b92460#npm:18.2.0"],\
["@primer/react", "virtual:ff5ad3439f8ec237c0c86796b437c422a681ce7f1211bc52c84c467fd5c19025673469e4b3bd047db74cf4144d670522e4013a081fcd63cc449dbcb3b5b92460#npm:35.20.0"],\
["@react-three/drei", "virtual:ff5ad3439f8ec237c0c86796b437c422a681ce7f1211bc52c84c467fd5c19025673469e4b3bd047db74cf4144d670522e4013a081fcd63cc449dbcb3b5b92460#npm:9.57.1"],\
["@react-three/fiber", "virtual:ff5ad3439f8ec237c0c86796b437c422a681ce7f1211bc52c84c467fd5c19025673469e4b3bd047db74cf4144d670522e4013a081fcd63cc449dbcb3b5b92460#npm:8.12.0"],\
@ -4213,6 +4214,13 @@ function $$SETUP_STATE(hydrateRuntimeState, basePath) {
],\
"linkType": "SOFT"\
}],\
["npm:18.2.0", {\
"packageLocation": "./.yarn/cache/@primer-octicons-react-npm-18.2.0-4922167f1d-95f1770f44.zip/node_modules/@primer/octicons-react/",\
"packageDependencies": [\
["@primer/octicons-react", "npm:18.2.0"]\
],\
"linkType": "SOFT"\
}],\
["virtual:02566b438c0e6066eb17807a9417eeeedfbeb730ff352bda25ea97131eff644f0590fd6ae4db6c892c79cc0e255483e826b0af8d4ec33f107675e1a30c98c4b8#npm:17.12.0", {\
"packageLocation": "./.yarn/__virtual__/@primer-octicons-react-virtual-d9b623413f/0/cache/@primer-octicons-react-npm-17.12.0-f2e14562a0-6af48465d2.zip/node_modules/@primer/octicons-react/",\
"packageDependencies": [\
@ -4225,6 +4233,19 @@ function $$SETUP_STATE(hydrateRuntimeState, basePath) {
"react"\
],\
"linkType": "HARD"\
}],\
["virtual:ff5ad3439f8ec237c0c86796b437c422a681ce7f1211bc52c84c467fd5c19025673469e4b3bd047db74cf4144d670522e4013a081fcd63cc449dbcb3b5b92460#npm:18.2.0", {\
"packageLocation": "./.yarn/__virtual__/@primer-octicons-react-virtual-dd8ad50232/0/cache/@primer-octicons-react-npm-18.2.0-4922167f1d-95f1770f44.zip/node_modules/@primer/octicons-react/",\
"packageDependencies": [\
["@primer/octicons-react", "virtual:ff5ad3439f8ec237c0c86796b437c422a681ce7f1211bc52c84c467fd5c19025673469e4b3bd047db74cf4144d670522e4013a081fcd63cc449dbcb3b5b92460#npm:18.2.0"],\
["@types/react", "npm:18.0.28"],\
["react", "npm:18.2.0"]\
],\
"packagePeers": [\
"@types/react",\
"react"\
],\
"linkType": "HARD"\
}]\
]],\
["@primer/primitives", [\
@ -8903,6 +8924,7 @@ function $$SETUP_STATE(hydrateRuntimeState, basePath) {
["@octokit/rest", "npm:19.0.7"],\
["@octokit/types", "npm:9.0.0"],\
["@playwright/test", "npm:1.31.2"],\
["@primer/octicons-react", "virtual:ff5ad3439f8ec237c0c86796b437c422a681ce7f1211bc52c84c467fd5c19025673469e4b3bd047db74cf4144d670522e4013a081fcd63cc449dbcb3b5b92460#npm:18.2.0"],\
["@primer/react", "virtual:ff5ad3439f8ec237c0c86796b437c422a681ce7f1211bc52c84c467fd5c19025673469e4b3bd047db74cf4144d670522e4013a081fcd63cc449dbcb3b5b92460#npm:35.20.0"],\
["@react-three/drei", "virtual:ff5ad3439f8ec237c0c86796b437c422a681ce7f1211bc52c84c467fd5c19025673469e4b3bd047db74cf4144d670522e4013a081fcd63cc449dbcb3b5b92460#npm:9.57.1"],\
["@react-three/fiber", "virtual:ff5ad3439f8ec237c0c86796b437c422a681ce7f1211bc52c84c467fd5c19025673469e4b3bd047db74cf4144d670522e4013a081fcd63cc449dbcb3b5b92460#npm:8.12.0"],\

1
package.json
View File

@ -8,6 +8,7 @@
"@octokit/openapi-types": "^16.0.0",
"@octokit/rest": "^19.0.7",
"@octokit/types": "^9.0.0",
"@primer/octicons-react": "^18.2.0",
"@primer/react": "^35.20.0",
"@react-three/drei": "^9.57.1",
"@react-three/fiber": "^8.11.1",

2
src/chrome/web.ts
View File

@ -71,7 +71,7 @@ export function mapInjectableDiffElements(
if (supportedElements.length !== supportedFiles.length) {
throw Error(
`elements and files have different length. Got ${supportedElements.length} and ${files.length}`
`elements and files have different length. Got ${supportedElements.length} and ${supportedFiles.length}`
)
}

6
src/components/diff/CadDiff.test.tsx
View File

@ -1,6 +0,0 @@
import { render } from '@testing-library/react'
import { CadDiff } from './CadDiff'
it('renders the CAD diff element', () => {
render(<CadDiff />)
})

72
src/components/diff/CadDiffPage.tsx
View File

@ -1,10 +1,11 @@
import React, { useEffect, useState } from 'react'
import '@react-three/fiber'
import { ThemeProvider } from '@primer/react'
import { Box, ThemeProvider } from '@primer/react'
import { DiffEntry, FileDiff, MessageIds } from '../../chrome/types'
import { createPortal } from 'react-dom'
import { Loading } from '../Loading'
import { CadDiff } from './CadDiff'
import { SourceRichToggle } from './SourceRichToggle'
function CadDiffPortal({
element,
@ -21,17 +22,28 @@ function CadDiffPortal({
sha: string
parentSha: string
}): React.ReactElement {
const [diff, setDiff] = useState<FileDiff>()
const [diffElement, setDiffElement] = useState<HTMLElement>()
const [richDiff, setRichDiff] = useState<FileDiff>()
const [richSelected, setRichSelected] = useState(true)
const [toolbarContainer, setToolbarContainer] = useState<HTMLElement>()
const [diffContainer, setDiffContainer] = useState<HTMLElement>()
const [sourceElements, setSourceElements] = useState<HTMLElement[]>([])
useEffect(() => {
const diffElement = element.querySelector(
'.js-file-content'
) as HTMLElement
setDiffElement(diffElement)
// TODO: don't clean up once the rich/source toggle is added
for (const e of diffElement.childNodes) {
e.remove()
const toolbar = element.querySelector<HTMLElement>('.file-info')
if (toolbar != null) {
setToolbarContainer(toolbar)
}
const diff = element.querySelector<HTMLElement>('.js-file-content')
if (diff != null) {
setDiffContainer(diff)
const sourceElements = Array.from(diff.children) as HTMLElement[]
sourceElements.map(n => (n.style.display = 'none'))
setSourceElements(sourceElements)
}
}, [element])
useEffect(() => {
;(async () => {
const response = await chrome.runtime.sendMessage({
id: MessageIds.GetFileDiff,
@ -40,20 +52,42 @@ function CadDiffPortal({
if ('error' in response) {
console.log(response.error)
} else {
setDiff(response as FileDiff)
setRichDiff(response as FileDiff)
}
})()
}, [element, diffElement, file, owner, repo, sha, parentSha])
}, [file, owner, repo, sha, parentSha])
return (
<>
{diffElement &&
{toolbarContainer &&
createPortal(
diff ? (
<CadDiff before={diff.before} after={diff.after} />
) : (
<Loading />
),
diffElement
<SourceRichToggle
disabled={!richDiff}
richSelected={richSelected}
onSourceSelected={() => {
sourceElements.map(n => (n.style.display = 'block'))
setRichSelected(false)
}}
onRichSelected={() => {
sourceElements.map(n => (n.style.display = 'none'))
setRichSelected(true)
}}
/>,
toolbarContainer
)}
{diffContainer &&
createPortal(
<Box sx={{ display: richSelected ? 'block' : 'none' }}>
{richDiff ? (
<CadDiff
before={richDiff.before}
after={richDiff.after}
/>
) : (
<Loading />
)}
</Box>,
diffContainer
)}
</>
)

35
src/components/diff/SourceRichToggle.test.tsx Normal file
View File

@ -0,0 +1,35 @@
import { fireEvent, render, screen } from '@testing-library/react'
import { SourceRichToggle } from './SourceRichToggle'
it('renders a source-rich diff toggle and checks its callbacks', async () => {
const callbackSource = jest.fn()
const callbackRich = jest.fn()
render(
<SourceRichToggle
disabled={false}
richSelected={true}
onSourceSelected={callbackSource}
onRichSelected={callbackRich}
/>
)
const [sourceButton, richButton] = await screen.findAllByRole('button')
expect(callbackSource.mock.calls).toHaveLength(0)
expect(callbackRich.mock.calls).toHaveLength(0)
fireEvent.click(sourceButton)
expect(callbackSource.mock.calls).toHaveLength(1)
fireEvent.click(richButton)
expect(callbackRich.mock.calls).toHaveLength(1)
})
it('renders a disbaled source-rich diff toggle', async () => {
render(<SourceRichToggle disabled={true} richSelected={true} />)
const [sourceButton, richButton] = await screen.findAllByRole('button')
expect(sourceButton).toBeDisabled()
expect(richButton).toBeDisabled()
})

59
src/components/diff/SourceRichToggle.tsx Normal file
View File

@ -0,0 +1,59 @@
import { ButtonGroup, IconButton, Tooltip } from '@primer/react'
import { PackageIcon, CodeIcon } from '@primer/octicons-react'
export type SourceRichToggleProps = {
disabled: boolean
richSelected: boolean
onSourceSelected?: () => void
onRichSelected?: () => void
}
export function SourceRichToggle({
disabled,
richSelected,
onSourceSelected,
onRichSelected,
}: SourceRichToggleProps) {
const commonButtonSx = {
color: 'fg.subtle',
width: '40px',
}
const commonTooltipSx = {
height: '32px',
}
const sourceText = 'Display the source diff'
const richText = 'Display the rich diff'
return (
<ButtonGroup sx={{ float: 'right', mr: '-8px' }}>
<Tooltip aria-label={sourceText} direction="w" sx={commonTooltipSx}>
<IconButton
aria-label={sourceText}
icon={CodeIcon}
disabled={disabled}
onClick={onSourceSelected}
sx={{
...commonButtonSx,
bg: !richSelected ? 'transparent' : 'neutral.subtle',
borderTopRightRadius: 0,
borderBottomRightRadius: 0,
borderRight: 'none',
}}
/>
</Tooltip>
<Tooltip aria-label={richText} direction="w" sx={commonTooltipSx}>
<IconButton
aria-label={sourceText}
icon={PackageIcon}
disabled={disabled}
onClick={onRichSelected}
sx={{
...commonButtonSx,
bg: richSelected ? 'transparent' : 'neutral.subtle',
borderTopLeftRadius: 0,
borderBottomLeftRadius: 0,
}}
/>
</Tooltip>
</ButtonGroup>
)
}

35
tests/extension.spec.ts
View File

@ -1,3 +1,4 @@
import { Page } from '@playwright/test'
import { test, expect } from './fixtures'
test('popup page', async ({ page, extensionId }) => {
@ -17,16 +18,29 @@ test('authorized popup page', async ({
await expect(page.locator('button')).toHaveCount(2)
})
async function getFirstDiffScreenshot(page: Page, url: string) {
page.on('console', msg => console.log(msg.text()))
await page.goto(url)
// waiting for the canvas (that holds the diff) to show up
await page.waitForSelector(
'.js-file[data-file-type=".obj"] .js-file-content canvas'
)
// screenshot the file diff with its toolbar
const element = await page.waitForSelector(
'.js-file[data-file-type=".obj"]'
)
await page.waitForTimeout(1000) // making sure the element fully settled in
return await element.screenshot()
}
test('pull request diff with an .obj file', async ({
page,
authorizedBackground,
}) => {
page.on('console', msg => console.log(msg.text()))
await page.goto('https://github.com/KittyCAD/kittycad.ts/pull/3/files')
const element = await page.waitForSelector('.js-file-content canvas')
await page.waitForTimeout(1000) // making sure the element fully settled in
const screenshot = await element.screenshot()
const url = 'https://github.com/KittyCAD/kittycad.ts/pull/3/files'
const screenshot = await getFirstDiffScreenshot(page, url)
expect(screenshot).toMatchSnapshot()
})
@ -34,13 +48,8 @@ test('commit diff with an .obj file', async ({
page,
authorizedBackground,
}) => {
page.on('console', msg => console.log(msg.text()))
await page.goto(
const url =
'https://github.com/KittyCAD/kittycad.ts/commit/08b50ee5a23b3ae7dd7b19383f14bbd520079cc1'
)
const element = await page.waitForSelector('.js-file-content canvas')
await page.waitForTimeout(1000) // making sure the element fully settled in
const screenshot = await element.screenshot()
const screenshot = await getFirstDiffScreenshot(page, url)
expect(screenshot).toMatchSnapshot()
})

BIN
tests/extension.spec.ts-snapshots/commit-diff-with-an-obj-file-1-chromium-darwin.png
View File

Binary file not shown.
Side by Side Swipe Overlay

Before

Width:  |  Height:  |  Size: 44 KiB

After

Width:  |  Height:  |  Size: 48 KiB

BIN
tests/extension.spec.ts-snapshots/commit-diff-with-an-obj-file-1-chromium-linux.png
View File

Binary file not shown.
Side by Side Swipe Overlay

Before

Width:  |  Height:  |  Size: 23 KiB

After

Width:  |  Height:  |  Size: 28 KiB

BIN
tests/extension.spec.ts-snapshots/pull-request-diff-with-an-obj-file-1-chromium-darwin.png
View File

Binary file not shown.
Side by Side Swipe Overlay

Before

Width:  |  Height:  |  Size: 39 KiB

After

Width:  |  Height:  |  Size: 44 KiB

BIN
tests/extension.spec.ts-snapshots/pull-request-diff-with-an-obj-file-1-chromium-linux.png
View File

Binary file not shown.
Side by Side Swipe Overlay

Before

Width:  |  Height:  |  Size: 23 KiB

After

Width:  |  Height:  |  Size: 28 KiB

10
yarn.lock
View File

@ -2524,6 +2524,15 @@ __metadata:
languageName: node
linkType: hard
"@primer/octicons-react@npm:^18.2.0":
version: 18.2.0
resolution: "@primer/octicons-react@npm:18.2.0"
peerDependencies:
react: ">=15"
checksum: 95f1770f44628b3e0fe93133a89c615a14c2353af6074cf172e95200dd553c4171db418ce858dbbd6b5699431a5e51007cce325207836e209f6fe3ead0ba3758
languageName: node
linkType: hard
"@primer/primitives@npm:7.10.0":
version: 7.10.0
resolution: "@primer/primitives@npm:7.10.0"
@ -6089,6 +6098,7 @@ __metadata:
"@octokit/rest": ^19.0.7
"@octokit/types": ^9.0.0
"@playwright/test": ^1.31.2
"@primer/octicons-react": ^18.2.0
"@primer/react": ^35.20.0
"@react-three/drei": ^9.57.1
"@react-three/fiber": ^8.11.1