-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: search more than only the notebook title #17379
Conversation
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just left a couple more comments but this seems much better. Feels really close now❗
frontend/src/scenes/notebooks/Nodes/NotebookNodeReplayTimestamp.tsx
Outdated
Show resolved
Hide resolved
@@ -75,6 +75,25 @@ export const NotebookNodeReplayTimestamp = Node.create({ | |||
group: 'inline', | |||
atom: true, | |||
|
|||
serializedText: | |||
() => | |||
(attrs: { playbackTime: number }): string => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the types not get carried over here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that there was already an interface, so can use that... but (I think) there's no type detection here because we're not wrapped in the create widget function that carries the type
Co-authored-by: David Newell <[email protected]>
Co-authored-by: David Newell <[email protected]>
Co-authored-by: David Newell <[email protected]>
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -79,6 +79,10 @@ export const NotebookNodeImage = createPostHogWidgetNode<NotebookNodeImageAttrib | |||
nodeType: NotebookNodeType.Image, | |||
title: 'Image', | |||
Component, | |||
serializedText: (attrs) => { | |||
// TODO file is null when this runs... should it be? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the right thing to do here might be to set the title to be the name of the file when updateAttributes
is called after uploading the file, and then the serialized text could just be attrs.title
. That's for a follow up though
Extends notebook search to cover all text content in the notebook
## TODO