Skip to content

Commit

Permalink
Properly remove in-page dashboard from DOM on close
Browse files Browse the repository at this point in the history
- Previous was leaving it in the DOM, causing a new one to be appended each time it got opened
- Most of the changes are renaming the `destroyInPageSidebarUI` fn, which was not specific to sidebar
  • Loading branch information
poltak authored and blackforestboi committed Apr 1, 2024
1 parent 17a11bf commit 2ee9182
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 37 deletions.
11 changes: 6 additions & 5 deletions src/content-scripts/content_script/sidebar.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { IGNORE_CLICK_OUTSIDE_CLASS } from '../constants'
import { ContentScriptRegistry, SidebarScriptMain } from './types'
import { createInPageUI, destroyInPageUI } from 'src/in-page-ui/utils'
import {
setupInPageSidebarUI,
destroyInPageSidebarUI,
} from 'src/sidebar/annotations-sidebar/index'
createInPageUI,
unmountInPageUI,
destroyInPageUI,
} from 'src/in-page-ui/utils'
import { setupInPageSidebarUI } from 'src/sidebar/annotations-sidebar/index'
import browser from 'webextension-polyfill'
import type { InPageUIRootMount } from 'src/in-page-ui/types'
import type { ShouldSetUpOptions } from 'src/in-page-ui/shared-state/types'
Expand Down Expand Up @@ -53,7 +54,7 @@ export const main: SidebarScriptMain = async (dependencies) => {
return
}
destroyInPageUI('sidebar')
destroyInPageSidebarUI(mount.rootElement, mount.shadowRoot)
unmountInPageUI(mount.rootElement, mount.shadowRoot)
}
}

Expand Down
27 changes: 14 additions & 13 deletions src/in-page-ui/dom.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { injectCSS } from 'src/util/content-injection'
import { runtime } from 'webextension-polyfill'
import type { InPageUIRootMount } from './types'

export function createInPageUIRoot({
containerId,
Expand All @@ -13,7 +14,7 @@ export function createInPageUIRoot({
cssFile?: string
rootClassNames?: string[]
containerClassNames?: string[]
}) {
}): InPageUIRootMount {
const container = document.createElement('div')
container.setAttribute('id', containerId)

Expand Down Expand Up @@ -91,7 +92,7 @@ const rootDirectory = runtime.getURL('/fonts/Satoshi/')
const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-Variable.woff) format('woff'),
url(${rootDirectory}Satoshi-Variable.ttf) format('truetype');
font-weight: 300;
Expand All @@ -101,7 +102,7 @@ const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-VariableItalic.woff) format('woff'),
url(${rootDirectory}Satoshi-VariableItalic.ttf) format('truetype');
font-weight: 300;
Expand All @@ -111,7 +112,7 @@ const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-Light.woff) format('woff'),
url(${rootDirectory}Satoshi-Light.ttf) format('truetype');
font-weight: 300;
Expand All @@ -121,7 +122,7 @@ const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-LightItalic.woff) format('woff'),
url(${rootDirectory}Satoshi-LightItalic.ttf) format('truetype');
font-weight: 300;
Expand All @@ -131,7 +132,7 @@ const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-Regular.woff) format('woff'),
url(${rootDirectory}Satoshi-Regular.ttf) format('truetype');
font-weight: 400;
Expand All @@ -141,7 +142,7 @@ const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-Italic.woff) format('woff'),
url(${rootDirectory}Satoshi-Italic.ttf) format('truetype');
font-weight: 400;
Expand All @@ -151,7 +152,7 @@ const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-Medium.woff) format('woff'),
url(${rootDirectory}Satoshi-Medium.ttf) format('truetype');
font-weight: 500;
Expand All @@ -161,7 +162,7 @@ const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-MediumItalic.woff) format('woff'),
url(${rootDirectory}Satoshi-MediumItalic.ttf) format('truetype');
font-weight: 500;
Expand All @@ -171,7 +172,7 @@ const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-Bold.woff) format('woff'),
url(${rootDirectory}Satoshi-Bold.ttf) format('truetype');
font-weight: 700;
Expand All @@ -181,7 +182,7 @@ const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-BoldItalic.woff) format('woff'),
url(${rootDirectory}Satoshi-BoldItalic.ttf) format('truetype');
font-weight: 700;
Expand All @@ -191,7 +192,7 @@ const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-Black.woff) format('woff'),
url(${rootDirectory}Satoshi-Black.ttf) format('truetype');
font-weight: 900;
Expand All @@ -201,7 +202,7 @@ const styles = `
@font-face {
font-family: 'Satoshi';
src:
src:
url(${rootDirectory}Satoshi-BlackItalic.woff) format('woff'),
url(${rootDirectory}Satoshi-BlackItalic.ttf) format('truetype');
font-weight: 900;
Expand Down
12 changes: 11 additions & 1 deletion src/in-page-ui/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// import retargetEvents from 'react-shadow-dom-retarget-events'
import ReactDOM from 'react-dom'
import { createInPageUIRoot, destroyRootElement } from './dom'

export function createInPageUI(
Expand All @@ -22,3 +22,13 @@ export function createInPageUI(
export function destroyInPageUI(name: string) {
return destroyRootElement(`memex-${name}-container`)
}

export function unmountInPageUI(target: HTMLElement, shadowRoot?: ShadowRoot) {
ReactDOM.unmountComponentAtNode(target)

if (shadowRoot) {
shadowRoot.removeChild(target)
} else {
document.body.removeChild(target)
}
}
12 changes: 7 additions & 5 deletions src/search-injection/search-display.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import {
import type { DashboardDependencies } from 'src/dashboard-refactor/types'
import { DashboardContainer } from 'src/dashboard-refactor'
import { InPageSearchModal } from '@worldbrain/memex-common/lib/common-ui/components/inPage-search-modal'
import { createInPageUI } from 'src/in-page-ui/utils'
import {
createInPageUI,
destroyInPageUI,
unmountInPageUI,
} from 'src/in-page-ui/utils'

type RootProps = Omit<DashboardDependencies, 'theme' | 'openSpaceInWebUI'> & {
rootEl: HTMLElement
Expand All @@ -30,10 +34,8 @@ class Root extends React.PureComponent<RootProps, RootState> {
}

private removeRoot = () => {
const unmountResult = ReactDOM.unmountComponentAtNode(this.props.rootEl)
if (unmountResult) {
this.props.rootEl.remove()
}
destroyInPageUI('search-display')
unmountInPageUI(this.props.rootEl, this.props.shadowRoot)
}

render() {
Expand Down
13 changes: 0 additions & 13 deletions src/sidebar/annotations-sidebar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,3 @@ export function setupInPageSidebarUI(
mount.rootElement,
)
}

export function destroyInPageSidebarUI(
target: HTMLElement,
shadowRoot?: ShadowRoot,
) {
ReactDOM.unmountComponentAtNode(target)

if (shadowRoot) {
shadowRoot.removeChild(target)
} else {
document.body.removeChild(target)
}
}

0 comments on commit 2ee9182

Please sign in to comment.