From 6e3ce74af653188213f44512e53acba1484e27bb Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 5 Oct 2023 15:25:33 +0200 Subject: [PATCH] feat: Notebook playlist comments (#17671) --- .../notebooks/Nodes/NotebookNodePlaylist.tsx | 22 ++++++- .../notebooks/Nodes/NotebookNodeRecording.tsx | 28 ++++++++- .../Nodes/NotebookNodeReplayTimestamp.tsx | 57 +++++++------------ .../Nodes/messaging/notebook-node-messages.ts | 27 +++++++++ .../notebooks/Nodes/notebookNodeLogic.ts | 38 ++++++++++++- .../src/scenes/notebooks/Notebook/Editor.tsx | 6 ++ .../notebooks/Notebook/notebookLogic.ts | 30 ++++++---- .../src/scenes/notebooks/Notebook/utils.ts | 1 + .../notebooks/Suggestions/ReplayTimestamp.tsx | 2 +- .../player/PlayerMetaLinks.tsx | 5 +- 10 files changed, 159 insertions(+), 57 deletions(-) create mode 100644 frontend/src/scenes/notebooks/Nodes/messaging/notebook-node-messages.ts diff --git a/frontend/src/scenes/notebooks/Nodes/NotebookNodePlaylist.tsx b/frontend/src/scenes/notebooks/Nodes/NotebookNodePlaylist.tsx index b25688bfa34d9..70700c8228a86 100644 --- a/frontend/src/scenes/notebooks/Nodes/NotebookNodePlaylist.tsx +++ b/frontend/src/scenes/notebooks/Nodes/NotebookNodePlaylist.tsx @@ -20,6 +20,7 @@ import { notebookNodeLogic } from './notebookNodeLogic' import { JSONContent, NotebookNodeViewProps, NotebookNodeAttributeProperties } from '../Notebook/utils' import { SessionRecordingsFilters } from 'scenes/session-recordings/filters/SessionRecordingsFilters' import { ErrorBoundary } from '@sentry/react' +import { sessionRecordingPlayerLogic } from 'scenes/session-recordings/player/sessionRecordingPlayerLogic' const Component = (props: NotebookNodeViewProps): JSX.Element => { const { filters, nodeId } = props.attributes @@ -41,10 +42,11 @@ const Component = (props: NotebookNodeViewProps) ) const { expanded } = useValues(notebookNodeLogic) - const { setActions, insertAfter } = useActions(notebookNodeLogic) + const { setActions, insertAfter, setMessageListeners, scrollIntoView } = useActions(notebookNodeLogic) const logic = sessionRecordingsListLogic(recordingPlaylistLogicProps) - const { activeSessionRecording, nextSessionRecording, matchingEventsMatchType } = useValues(logic) + const { activeSessionRecording, nextSessionRecording, matchingEventsMatchType, sessionRecordings } = + useValues(logic) const { setSelectedRecordingId } = useActions(logic) useEffect(() => { @@ -67,8 +69,22 @@ const Component = (props: NotebookNodeViewProps) ) }, [activeSessionRecording]) + useEffect(() => { + setMessageListeners({ + 'play-replay': ({ sessionRecordingId, time }) => { + setSelectedRecordingId(sessionRecordingId) + scrollIntoView() + + setTimeout(() => { + // NOTE: This is a hack but we need a delay to give time for the player to mount + sessionRecordingPlayerLogic.findMounted({ playerKey, sessionRecordingId })?.actions.seekToTime(time) + }, 100) + }, + }) + }, []) + if (!expanded) { - return
20+ recordings
+ return
{sessionRecordings.length}+ recordings
} const content = !activeSessionRecording?.id ? ( diff --git a/frontend/src/scenes/notebooks/Nodes/NotebookNodeRecording.tsx b/frontend/src/scenes/notebooks/Nodes/NotebookNodeRecording.tsx index 80789f5cdbb91..61e26de5e52d1 100644 --- a/frontend/src/scenes/notebooks/Nodes/NotebookNodeRecording.tsx +++ b/frontend/src/scenes/notebooks/Nodes/NotebookNodeRecording.tsx @@ -8,6 +8,7 @@ import { urls } from 'scenes/urls' import { SessionRecordingPlayerMode, getCurrentPlayerTime, + sessionRecordingPlayerLogic, } from 'scenes/session-recordings/player/sessionRecordingPlayerLogic' import { useActions, useValues } from 'kea' import { sessionRecordingDataLogic } from 'scenes/session-recordings/player/sessionRecordingDataLogic' @@ -37,10 +38,19 @@ const Component = (props: NotebookNodeViewProps noInspector: noInspector, } + const { expanded } = useValues(notebookNodeLogic) + const { + setActions, + insertAfter, + insertReplayCommentByTimestamp, + setMessageListeners, + setExpanded, + scrollIntoView, + } = useActions(notebookNodeLogic) + const { sessionPlayerMetaData } = useValues(sessionRecordingDataLogic(recordingLogicProps)) const { loadRecordingMeta } = useActions(sessionRecordingDataLogic(recordingLogicProps)) - const { expanded } = useValues(notebookNodeLogic) - const { setActions, insertAfter, insertReplayCommentByTimestamp } = useActions(notebookNodeLogic) + const { seekToTime, setPlay } = useActions(sessionRecordingPlayerLogic(recordingLogicProps)) useEffect(() => { loadRecordingMeta() @@ -76,6 +86,20 @@ const Component = (props: NotebookNodeViewProps ]) }, [sessionPlayerMetaData?.person?.id]) + useEffect(() => { + setMessageListeners({ + 'play-replay': ({ time }) => { + if (!expanded) { + setExpanded(true) + } + setPlay() + + seekToTime(time) + scrollIntoView() + }, + }) + }, []) + return !expanded ? (
{sessionPlayerMetaData ? ( diff --git a/frontend/src/scenes/notebooks/Nodes/NotebookNodeReplayTimestamp.tsx b/frontend/src/scenes/notebooks/Nodes/NotebookNodeReplayTimestamp.tsx index 88db6f4395ffc..032eef1110452 100644 --- a/frontend/src/scenes/notebooks/Nodes/NotebookNodeReplayTimestamp.tsx +++ b/frontend/src/scenes/notebooks/Nodes/NotebookNodeReplayTimestamp.tsx @@ -1,47 +1,39 @@ import { mergeAttributes, Node, NodeViewProps } from '@tiptap/core' import { NodeViewWrapper, ReactNodeViewRenderer } from '@tiptap/react' import { NotebookNodeType, NotebookTarget } from '~/types' -import { - sessionRecordingPlayerLogic, - SessionRecordingPlayerLogicProps, -} from 'scenes/session-recordings/player/sessionRecordingPlayerLogic' import { dayjs } from 'lib/dayjs' import { JSONContent } from '../Notebook/utils' import clsx from 'clsx' -import { findPositionOfClosestNodeMatchingAttrs } from '../Notebook/Editor' import { urls } from 'scenes/urls' import { LemonButton } from '@posthog/lemon-ui' import { notebookLogic } from '../Notebook/notebookLogic' import { useValues } from 'kea' -import { sessionRecordingPlayerProps } from './NotebookNodeRecording' import { useMemo } from 'react' import { openNotebook } from '~/models/notebooksModel' +export interface NotebookNodeReplayTimestampAttrs { + playbackTime?: number + sessionRecordingId: string + sourceNodeId?: string +} + const Component = (props: NodeViewProps): JSX.Element => { - const { shortId, findNodeLogic } = useValues(notebookLogic) - const sessionRecordingId: string = props.node.attrs.sessionRecordingId - const playbackTime: number = props.node.attrs.playbackTime + const { shortId, findNodeLogic, findNodeLogicById } = useValues(notebookLogic) + const { sessionRecordingId, playbackTime = 0, sourceNodeId } = props.node.attrs as NotebookNodeReplayTimestampAttrs - const recordingNodeInNotebook = useMemo(() => { - return findNodeLogic(NotebookNodeType.Recording, { id: sessionRecordingId }) + const relatedNodeInNotebook = useMemo(() => { + const logicById = sourceNodeId ? findNodeLogicById(sourceNodeId) : null + + return logicById ?? findNodeLogic(NotebookNodeType.Recording, { id: sessionRecordingId }) }, [findNodeLogic]) const handlePlayInNotebook = (): void => { - recordingNodeInNotebook?.actions.setExpanded(true) - - // TODO: Move all of the above into the logic / Node context for the recording node - const logicProps: SessionRecordingPlayerLogicProps = sessionRecordingPlayerProps(sessionRecordingId) - const logic = sessionRecordingPlayerLogic(logicProps) - - logic.actions.seekToTime(props.node.attrs.playbackTime) - logic.actions.setPlay() + // TODO: Figure out how to send this action info to the playlist OR the replay node... - const recordingNodePosition = findPositionOfClosestNodeMatchingAttrs(props.editor, props.getPos(), { - id: sessionRecordingId, + relatedNodeInNotebook?.values.sendMessage('play-replay', { + sessionRecordingId, + time: playbackTime ?? 0, }) - - const domEl = props.editor.view.nodeDOM(recordingNodePosition) as HTMLElement - domEl.scrollIntoView() } return ( @@ -55,10 +47,10 @@ const Component = (props: NodeViewProps): JSX.Element => { type="secondary" status="primary-alt" onClick={ - recordingNodeInNotebook ? handlePlayInNotebook : () => openNotebook(shortId, NotebookTarget.Popover) + relatedNodeInNotebook ? handlePlayInNotebook : () => openNotebook(shortId, NotebookTarget.Popover) } to={ - !recordingNodeInNotebook + !relatedNodeInNotebook ? urls.replaySingle(sessionRecordingId) + `?t=${playbackTime / 1000}` : undefined } @@ -85,6 +77,7 @@ export const NotebookNodeReplayTimestamp = Node.create({ return { playbackTime: { default: null, keepOnSplit: false }, sessionRecordingId: { default: null, keepOnSplit: true, isRequired: true }, + sourceNodeId: { default: null, keepOnSplit: true }, } }, @@ -105,21 +98,13 @@ export function formatTimestamp(time: number): string { return dayjs.duration(time, 'milliseconds').format('HH:mm:ss').replace(/^00:/, '').trim() } -export interface NotebookNodeReplayTimestampAttrs { - playbackTime: number | null - sessionRecordingId: string -} - -export function buildTimestampCommentContent( - currentPlayerTime: number | null, - sessionRecordingId: string -): JSONContent { +export function buildTimestampCommentContent(attrs: NotebookNodeReplayTimestampAttrs): JSONContent { return { type: 'paragraph', content: [ { type: NotebookNodeType.ReplayTimestamp, - attrs: { playbackTime: currentPlayerTime, sessionRecordingId: sessionRecordingId }, + attrs, }, { type: 'text', text: ' ' }, ], diff --git a/frontend/src/scenes/notebooks/Nodes/messaging/notebook-node-messages.ts b/frontend/src/scenes/notebooks/Nodes/messaging/notebook-node-messages.ts new file mode 100644 index 0000000000000..9a122e7daa7c6 --- /dev/null +++ b/frontend/src/scenes/notebooks/Nodes/messaging/notebook-node-messages.ts @@ -0,0 +1,27 @@ +/** + * NotebookNode messaging + * + * To help communication between nodes, you can register a message handler from any of the defined list. + * Typing wise it is tricky to scope so all events handlers are typed as possibly undefined. + */ + +export type NotebookNodeMessages = { + 'play-replay': { + sessionRecordingId: string + time: number + } + // Not used yet but as a future idea - you could "ping" a node to have it highlight or something. + ping: { + message: string + } +} + +export type NotebookNodeMessagesNames = keyof NotebookNodeMessages + +export type NotebookNodeMessagesListener = ( + e: NotebookNodeMessages[MessageName] +) => void + +export type NotebookNodeMessagesListeners = { + [MessageName in NotebookNodeMessagesNames]?: NotebookNodeMessagesListener +} diff --git a/frontend/src/scenes/notebooks/Nodes/notebookNodeLogic.ts b/frontend/src/scenes/notebooks/Nodes/notebookNodeLogic.ts index 28d481ced6096..d4c637fed3923 100644 --- a/frontend/src/scenes/notebooks/Nodes/notebookNodeLogic.ts +++ b/frontend/src/scenes/notebooks/Nodes/notebookNodeLogic.ts @@ -27,6 +27,7 @@ import { } from '../Notebook/utils' import { NotebookNodeType } from '~/types' import posthog from 'posthog-js' +import { NotebookNodeMessages, NotebookNodeMessagesListeners } from './messaging/notebook-node-messages' export type NotebookNodeLogicProps = { node: NotebookNode @@ -36,6 +37,7 @@ export type NotebookNodeLogicProps = { getPos: () => number resizeable: boolean | ((attributes: CustomNotebookNodeAttributes) => boolean) widgets: NotebookNodeWidget[] + messageListeners?: NotebookNodeMessagesListeners startExpanded: boolean } & NotebookNodeAttributeProperties @@ -63,6 +65,8 @@ export const notebookNodeLogic = kea([ setNextNode: (node: Node | null) => ({ node }), deleteNode: true, selectNode: true, + scrollIntoView: true, + setMessageListeners: (listeners: NotebookNodeMessagesListeners) => ({ listeners }), }), connect((props: NotebookNodeLogicProps) => ({ @@ -101,12 +105,35 @@ export const notebookNodeLogic = kea([ setActions: (_, { actions }) => actions.filter((x) => !!x) as NotebookNodeAction[], }, ], + messageListeners: [ + props.messageListeners as NotebookNodeMessagesListeners, + { + setMessageListeners: (_, { listeners }) => listeners, + }, + ], })), selectors({ notebookLogic: [(_, p) => [p.notebookLogic], (notebookLogic) => notebookLogic], nodeAttributes: [(_, p) => [p.attributes], (nodeAttributes) => nodeAttributes], widgets: [(_, p) => [p.widgets], (widgets) => widgets], + + sendMessage: [ + (s) => [s.messageListeners], + (messageListeners) => { + return ( + message: T, + payload: NotebookNodeMessages[T] + ): boolean => { + if (!messageListeners[message]) { + return false + } + + messageListeners[message]?.(payload) + return true + } + }, + ], }), listeners(({ actions, values, props }) => ({ @@ -142,6 +169,10 @@ export const notebookNodeLogic = kea([ } }, + scrollIntoView: () => { + values.editor?.scrollToPosition(props.getPos()) + }, + insertAfterLastNodeOfType: ({ nodeType, content }) => { const insertionPosition = props.getPos() values.notebookLogic.actions.insertAfterLastNodeOfType(nodeType, content, insertionPosition) @@ -149,11 +180,12 @@ export const notebookNodeLogic = kea([ insertReplayCommentByTimestamp: ({ timestamp, sessionRecordingId }) => { const insertionPosition = props.getPos() - values.notebookLogic.actions.insertReplayCommentByTimestamp( + values.notebookLogic.actions.insertReplayCommentByTimestamp({ timestamp, sessionRecordingId, - insertionPosition - ) + knownStartingPosition: insertionPosition, + nodeId: props.nodeId, + }) }, setExpanded: ({ expanded }) => { diff --git a/frontend/src/scenes/notebooks/Notebook/Editor.tsx b/frontend/src/scenes/notebooks/Notebook/Editor.tsx index e281656c343f7..7866a61617f87 100644 --- a/frontend/src/scenes/notebooks/Notebook/Editor.tsx +++ b/frontend/src/scenes/notebooks/Notebook/Editor.tsx @@ -216,6 +216,12 @@ export function Editor({ domEl.scrollIntoView({ behavior: 'smooth', block: 'start', inline: 'nearest' }) }) }, + scrollToPosition(position) { + queueMicrotask(() => { + const domEl = editor.view.nodeDOM(position) as HTMLElement + domEl.scrollIntoView({ behavior: 'smooth', block: 'start', inline: 'nearest' }) + }) + }, }) }, onUpdate: onUpdate, diff --git a/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts b/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts index 5162fb37b2b26..2aff49d981c4b 100644 --- a/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts +++ b/frontend/src/scenes/notebooks/Notebook/notebookLogic.ts @@ -95,16 +95,12 @@ export const notebookLogic = kea([ nodeType, knownStartingPosition, }), - insertReplayCommentByTimestamp: ( - timestamp: number, - sessionRecordingId: string, + insertReplayCommentByTimestamp: (options: { + timestamp: number + sessionRecordingId: string knownStartingPosition?: number - ) => ({ - timestamp, - sessionRecordingId, - // if operating on a particular instance of a replay comment, we can pass the known starting position - knownStartingPosition, - }), + nodeId?: string + }) => options, setShowHistory: (showHistory: boolean) => ({ showHistory }), }), reducers({ @@ -344,6 +340,14 @@ export const notebookLogic = kea([ } }, ], + findNodeLogicById: [ + (s) => [s.nodeLogics], + (nodeLogics) => { + return (id: string): notebookNodeLogicType | null => { + return Object.values(nodeLogics).find((nodeLogic) => nodeLogic.props.nodeId === id) ?? null + } + }, + ], isShowingSidebar: [ (s) => [s.editingNodeId, s.showHistory], @@ -403,7 +407,7 @@ export const notebookLogic = kea([ } ) }, - insertReplayCommentByTimestamp: async ({ timestamp, sessionRecordingId, knownStartingPosition }) => { + insertReplayCommentByTimestamp: async ({ timestamp, sessionRecordingId, knownStartingPosition, nodeId }) => { await runWhenEditorIsReady( () => !!values.editor, () => { @@ -424,7 +428,11 @@ export const notebookLogic = kea([ values.editor?.insertContentAfterNode( insertionPosition, - buildTimestampCommentContent(timestamp, sessionRecordingId) + buildTimestampCommentContent({ + playbackTime: timestamp, + sessionRecordingId, + sourceNodeId: nodeId, + }) ) } ) diff --git a/frontend/src/scenes/notebooks/Notebook/utils.ts b/frontend/src/scenes/notebooks/Notebook/utils.ts index 25a1f527abc1f..e0ad5b2396566 100644 --- a/frontend/src/scenes/notebooks/Notebook/utils.ts +++ b/frontend/src/scenes/notebooks/Notebook/utils.ts @@ -78,6 +78,7 @@ export interface NotebookEditor { nextNode: (position: number) => { node: Node; position: number } | null hasChildOfType: (node: Node, type: string) => boolean scrollToSelection: () => void + scrollToPosition: (position: number) => void } // Loosely based on https://github.com/ueberdosis/tiptap/blob/develop/packages/extension-floating-menu/src/floating-menu-plugin.ts#LL38C3-L55C4 diff --git a/frontend/src/scenes/notebooks/Suggestions/ReplayTimestamp.tsx b/frontend/src/scenes/notebooks/Suggestions/ReplayTimestamp.tsx index a36940e6c98d9..ce7c762d7bd66 100644 --- a/frontend/src/scenes/notebooks/Suggestions/ReplayTimestamp.tsx +++ b/frontend/src/scenes/notebooks/Suggestions/ReplayTimestamp.tsx @@ -22,7 +22,7 @@ const insertTimestamp = ({ sessionRecordingPlayerLogic.findMounted(sessionRecordingPlayerProps(sessionRecordingId))?.values .currentPlayerTime || 0 - editor.insertContent([buildTimestampCommentContent(currentPlayerTime, sessionRecordingId)]) + editor.insertContent([buildTimestampCommentContent({ playbackTime: currentPlayerTime, sessionRecordingId })]) } } diff --git a/frontend/src/scenes/session-recordings/player/PlayerMetaLinks.tsx b/frontend/src/scenes/session-recordings/player/PlayerMetaLinks.tsx index 451f1cf616f8a..8eabc0fbf452f 100644 --- a/frontend/src/scenes/session-recordings/player/PlayerMetaLinks.tsx +++ b/frontend/src/scenes/session-recordings/player/PlayerMetaLinks.tsx @@ -70,7 +70,10 @@ export function PlayerMetaLinks(): JSX.Element { return } - theNotebookLogic.actions.insertReplayCommentByTimestamp(time, sessionRecordingId) + theNotebookLogic.actions.insertReplayCommentByTimestamp({ + timestamp: time, + sessionRecordingId, + }) }} > Comment