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

chore: open / close notebook widgets #17375

Merged
merged 25 commits into from
Sep 15, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
923c095
chore: open / close notebook widgets
daibhin Sep 11, 2023
fbaa8d8
Update UI snapshots for `chromium` (2)
github-actions[bot] Sep 11, 2023
b306117
Merge branch 'master' into dn-chore/notebooks-widgets-toggling
daibhin Sep 12, 2023
5411813
Merge branch 'master' into dn-chore/notebooks-widgets-toggling
daibhin Sep 12, 2023
d5b7939
Update UI snapshots for `chromium` (2)
github-actions[bot] Sep 12, 2023
c7dade6
Update UI snapshots for `chromium` (1)
github-actions[bot] Sep 12, 2023
b7faf60
leave settings open
daibhin Sep 12, 2023
a836b78
hide if not custom node
daibhin Sep 12, 2023
6c4b3e1
Merge branch 'master' into dn-chore/notebooks-widgets-toggling
daibhin Sep 12, 2023
bba72cb
Merge branch 'master' into dn-chore/notebooks-widgets-toggling
daibhin Sep 13, 2023
675168a
Update UI snapshots for `chromium` (2)
github-actions[bot] Sep 13, 2023
af079ee
Merge branch 'master' into dn-chore/notebooks-widgets-toggling
daibhin Sep 13, 2023
7e190bb
Update UI snapshots for `chromium` (2)
github-actions[bot] Sep 13, 2023
d327e6e
Merge branch 'master' into dn-chore/notebooks-widgets-toggling
daibhin Sep 14, 2023
9d715a9
Update UI snapshots for `chromium` (1)
github-actions[bot] Sep 14, 2023
d2c7348
Merge branch 'master' into dn-chore/notebooks-widgets-toggling
daibhin Sep 14, 2023
63b3c0c
remove widget feature flag
daibhin Sep 14, 2023
fd883c7
Update UI snapshots for `chromium` (2)
github-actions[bot] Sep 14, 2023
f2fcbd3
Update UI snapshots for `chromium` (1)
github-actions[bot] Sep 14, 2023
1ff763d
Merge branch 'master' into dn-chore/notebooks-widgets-toggling
daibhin Sep 14, 2023
33d201d
add toggle support & active state
daibhin Sep 14, 2023
632b16e
Merge branch 'master' into dn-chore/notebooks-widgets-toggling
daibhin Sep 15, 2023
da3cc83
Update UI snapshots for `chromium` (1)
github-actions[bot] Sep 15, 2023
4067347
Update UI snapshots for `chromium` (2)
github-actions[bot] Sep 15, 2023
fefb59e
Update UI snapshots for `chromium` (1)
github-actions[bot] Sep 15, 2023
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion frontend/src/lib/lemon-ui/LemonWidget/LemonWidget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function LemonWidget({ title, collapsible = true, onClose, children }: Le
/>
</>
) : (
<span className="flex-1">{title}</span>
<span className="flex-1 text-primary-alt px-2">{title}</span>
)}
{onClose && <LemonButton status="danger" onClick={onClose} size="small" icon={<IconClose />} />}
</Header>
Expand Down
7 changes: 4 additions & 3 deletions frontend/src/scenes/notebooks/Nodes/NodeWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export function NodeWrapper<T extends CustomNotebookNodeAttributes>({
}: NodeWrapperProps<T> & NotebookNodeViewProps<T>): JSX.Element {
const mountedNotebookLogic = useMountedLogic(notebookLogic)
const { isEditable } = useValues(mountedNotebookLogic)
const { setIsShowingSidebar } = useActions(mountedNotebookLogic)

// nodeId can start null, but should then immediately be generated
const nodeId = node.attrs.nodeId
Expand All @@ -84,7 +85,7 @@ export function NodeWrapper<T extends CustomNotebookNodeAttributes>({
}
const nodeLogic = useMountedLogic(notebookNodeLogic(nodeLogicProps))
const { title, resizeable, expanded } = useValues(nodeLogic)
const { setExpanded, deleteNode, setWidgetsVisible } = useActions(nodeLogic)
const { setExpanded, deleteNode } = useActions(nodeLogic)

const [ref, inView] = useInView({ triggerOnce: true })
const contentRef = useRef<HTMLDivElement | null>(null)
Expand Down Expand Up @@ -163,9 +164,9 @@ export function NodeWrapper<T extends CustomNotebookNodeAttributes>({
/>
)}

{!!widgets.length && isEditable ? (
{widgets.length > 0 ? (
<LemonButton
onClick={() => setWidgetsVisible(true)}
onClick={() => setIsShowingSidebar(true)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this should toggle, rather than just showing. And it would be great if it could have an "active" state if open.

size="small"
icon={<IconFilter />}
/>
Expand Down
3 changes: 1 addition & 2 deletions frontend/src/scenes/notebooks/Nodes/NotebookNodePlaylist.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { SessionRecordingPlayer } from 'scenes/session-recordings/player/Session
import { useMemo, useRef, useState } from 'react'
import { fromParamsGivenUrl, uuid } from 'lib/utils'
import { LemonButton } from '@posthog/lemon-ui'
import { IconChevronLeft, IconSettings } from 'lib/lemon-ui/icons'
import { IconChevronLeft } from 'lib/lemon-ui/icons'
import { urls } from 'scenes/urls'
import { notebookNodeLogic } from './notebookNodeLogic'
import { JSONContent, NotebookNodeViewProps, NotebookNodeWidgetSettings } from '../Notebook/utils'
Expand Down Expand Up @@ -135,7 +135,6 @@ export const NotebookNodePlaylist = createPostHogWidgetNode<NotebookNodePlaylist
{
key: 'settings',
label: 'Settings',
icon: <IconSettings />,
Component: Settings,
},
],
Expand Down
2 changes: 0 additions & 2 deletions frontend/src/scenes/notebooks/Nodes/NotebookNodeQuery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { useMemo } from 'react'
import { notebookNodeLogic } from './notebookNodeLogic'
import { NotebookNodeViewProps, NotebookNodeWidgetSettings } from '../Notebook/utils'
import clsx from 'clsx'
import { IconSettings } from 'lib/lemon-ui/icons'
import { urls } from 'scenes/urls'
import api from 'lib/api'

Expand Down Expand Up @@ -133,7 +132,6 @@ export const NotebookNodeQuery = createPostHogWidgetNode<NotebookNodeQueryAttrib
{
key: 'settings',
label: 'Settings',
icon: <IconSettings />,
Component: Settings,
},
],
Expand Down
2 changes: 0 additions & 2 deletions frontend/src/scenes/notebooks/Nodes/NotebookNodeRecording.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
} from 'scenes/session-recordings/playlist/SessionRecordingPreview'
import { notebookNodeLogic } from './notebookNodeLogic'
import { LemonSwitch } from '@posthog/lemon-ui'
import { IconSettings } from 'lib/lemon-ui/icons'
import { JSONContent, NotebookNodeViewProps, NotebookNodeWidgetSettings } from '../Notebook/utils'

const HEIGHT = 500
Expand Down Expand Up @@ -102,7 +101,6 @@ export const NotebookNodeRecording = createPostHogWidgetNode<NotebookNodeRecordi
{
key: 'settings',
label: 'Settings',
icon: <IconSettings />,
Component: Settings,
},
],
Expand Down
11 changes: 0 additions & 11 deletions frontend/src/scenes/notebooks/Nodes/notebookNodeLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ export const notebookNodeLogic = kea<notebookNodeLogicType>([
timestamp,
sessionRecordingId,
}),
setWidgetsVisible: (visible: boolean) => ({ visible }),
setPreviousNode: (node: Node | null) => ({ node }),
setNextNode: (node: Node | null) => ({ node }),
deleteNode: true,
Expand Down Expand Up @@ -106,22 +105,12 @@ export const notebookNodeLogic = kea<notebookNodeLogicType>([
setNextNode: (_, { node }) => node,
},
],
widgetsVisible: [
false,
{
setWidgetsVisible: (_, { visible }) => visible,
},
],
})),

selectors({
notebookLogic: [(_, p) => [p.notebookLogic], (notebookLogic) => notebookLogic],
nodeAttributes: [(_, p) => [p.nodeAttributes], (nodeAttributes) => nodeAttributes],
widgets: [(_, p) => [p.widgets], (widgets) => widgets],
isShowingWidgets: [
(s, p) => [s.widgetsVisible, p.widgets],
(widgetsVisible, widgets) => !!widgets.length && widgetsVisible,
],
}),

listeners(({ actions, values, props }) => ({
Expand Down
25 changes: 16 additions & 9 deletions frontend/src/scenes/notebooks/Notebook/NotebookSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { notebookNodeLogicType } from '../Nodes/notebookNodeLogicType'

export const NotebookSidebar = (): JSX.Element | null => {
const { selectedNodeLogic, isShowingSidebar, isEditable } = useValues(notebookLogic)
const { setIsShowingSidebar } = useActions(notebookLogic)

if (!isEditable) {
return null
Expand All @@ -17,23 +18,29 @@ export const NotebookSidebar = (): JSX.Element | null => {
'NotebookSidebar--showing': isShowingSidebar,
})}
>
<div className="NotebookSidebar__content">{selectedNodeLogic && <Widgets logic={selectedNodeLogic} />}</div>
<div className="NotebookSidebar__content">
{selectedNodeLogic && isShowingSidebar && (
<Widgets logic={selectedNodeLogic} onClose={() => setIsShowingSidebar(false)} />
)}
</div>
</div>
)
}

export const Widgets = ({ logic }: { logic: BuiltLogic<notebookNodeLogicType> }): JSX.Element | null => {
const { widgets, nodeAttributes, isShowingWidgets } = useValues(logic)
const { updateAttributes, setWidgetsVisible } = useActions(logic)

if (!isShowingWidgets) {
return null
}
export const Widgets = ({
logic,
onClose,
}: {
logic: BuiltLogic<notebookNodeLogicType>
onClose: () => void
}): JSX.Element | null => {
const { widgets, nodeAttributes } = useValues(logic)
const { updateAttributes } = useActions(logic)

return (
<div className="NotebookNodeSettings__widgets space-y-2 w-full">
{widgets.map(({ key, label, Component }) => (
<LemonWidget key={key} title={label} onClose={() => setWidgetsVisible(false)}>
<LemonWidget key={key} title={label} collapsible={false} onClose={onClose}>
<Component attributes={nodeAttributes} updateAttributes={updateAttributes} />
</LemonWidget>
))}
Expand Down
12 changes: 8 additions & 4 deletions frontend/src/scenes/notebooks/Notebook/notebookLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export const notebookLogic = kea<notebookLogicType>([
exportJSON: true,
showConflictWarning: true,
onUpdateEditor: true,
setIsShowingSidebar: (showing: boolean) => ({ showing }),
registerNodeLogic: (nodeLogic: BuiltLogic<notebookNodeLogicType>) => ({ nodeLogic }),
unregisterNodeLogic: (nodeLogic: BuiltLogic<notebookNodeLogicType>) => ({ nodeLogic }),
setEditable: (editable: boolean) => ({ editable }),
Expand Down Expand Up @@ -162,6 +163,13 @@ export const notebookLogic = kea<notebookLogicType>([
setEditable: (_, { editable }) => editable,
},
],
isShowingSidebar: [
false,
{
setSelectedNodeId: () => false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be here anymore. If we are changing to a user initiated opening then I think we should leave it open until either the setting is closed or the source node is deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I made a change to leave the sidebar open except in the case when you click a non-custom node (e.g. paragraph)

setIsShowingSidebar: (_, { showing }) => showing,
},
],
}),
loaders(({ values, props, actions }) => ({
notebook: [
Expand Down Expand Up @@ -327,10 +335,6 @@ export const notebookLogic = kea<notebookLogicType>([
}
},
],
isShowingSidebar: [
(s) => [s.selectedNodeLogic],
(selectedNodeLogic) => selectedNodeLogic?.values.isShowingWidgets,
],
}),
sharedListeners(({ values, actions }) => ({
onNotebookChange: () => {
Expand Down
1 change: 0 additions & 1 deletion frontend/src/scenes/notebooks/Notebook/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export type NotebookNodeViewProps<T extends CustomNotebookNodeAttributes> = Omit
export type NotebookNodeWidget = {
key: string
label: string
icon: JSX.Element
// using 'any' here shouldn't be necessary but I couldn't figure out how to set a generic on the notebookNodeLogic props
Component: ({ attributes, updateAttributes }: NotebookNodeWidgetSettings<any>) => JSX.Element
}
Expand Down