feat(flows): draggable visualizations #18486
Conversation
| register({ | ||
| type: 'query', | ||
| component: View, | ||
| button: 'Custom Script', | ||
| initial: { | ||
| resultsVisibility: 'visible', | ||
| resultsPanelHeight: 200, | ||
| panelVisibility: 'visible', |
There was a problem hiding this comment.
this resizing is a weird concept in here. like, is it notebook specific? but it's not shared by all panels... and a panel in the query context is a child of the notebook panel, but in a visualization, it's the panel itself? most of my hangups come from not having a set of clear definitions around this
There was a problem hiding this comment.
Same same. I briefly considered moving it into meta or even its own context but did not because only some types would use it
| resizingEnabled={!!results.raw} | ||
| emptyText="No data to visualize" | ||
| emptyIcon={IconFont.BarChart} | ||
| toggleVisibilityEnabled={false} |
| /** When resizing is enabled the panel cannot be resized below this amount */ | ||
| minimumHeight?: number | ||
| /** Renders this element beneath the visibility toggle in the header */ | ||
| additionalControls?: JSX.Element | JSX.Element[] |
There was a problem hiding this comment.
the idea being we're gonna add a gear icon for view options? it be better to have a more rigid interface description here if that's the case. like an array of "icon, enabled, onclick, hover text" if it's just all going to be the same buttons, with small differences?
| <div className="panel-resizer"> | ||
| <ResizerHeader | ||
| emptyIcon={emptyIcon} | ||
| visibility={visibility} |
There was a problem hiding this comment.
this shouldn't be passed down
There was a problem hiding this comment.
note: this is going to be addressed in a clean up pass when adding the "CurrentPipe" context
| <ResizerHeader | ||
| emptyIcon={emptyIcon} | ||
| visibility={visibility} | ||
| onStartDrag={handleMouseDown} |
There was a problem hiding this comment.
why handle the drag logic here, and not in the child component?
| dragHandleRef={dragHandleRef} | ||
| resizingEnabled={resizingEnabled} | ||
| additionalControls={additionalControls} | ||
| onUpdateVisibility={handleUpdateVisibility} |
There was a problem hiding this comment.
this shouldn't be passed down
There was a problem hiding this comment.
note: this is going to be addressed in a clean up pass when adding the "CurrentPipe" context
|
there's a lot of prop drilling going on, can we tighten that up? would it also help if we created a "CurrentPipe" context and provider for all of the NotebookPipes? then we can decouple that drilling even more be removing the "data/onUpdate" props |
ResizerandResizerHandleinto generic components and move tonotebooks/shared/Visualization without data:
