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.

Sign up for GitHub

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

Jump to bottom

Reconnect visualizations when switching their types #11494

Merged
merged 8 commits into from
Nov 13, 2024
Merged

Reconnect visualizations when switching their types #11494

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
eb18e80
Reconnect visualizations when the type changes
vitvakatu Nov 5, 2024
33595d5
Merge branch 'develop' into wip/vitvakatu/vis-updates
vitvakatu Nov 5, 2024
caaab4a
Update vue devtools
vitvakatu Nov 7, 2024
6bc9d56
debugAccess utility
vitvakatu Nov 7, 2024
e01854c
Fix error visualization failure and rework visualization reconnection
vitvakatu Nov 7, 2024
81a801f
bring back onCleanup
vitvakatu Nov 7, 2024
d41a8be
Merge branch 'develop' into wip/vitvakatu/vis-updates
vitvakatu Nov 12, 2024
588ec37
Merge branch 'develop' into wip/vitvakatu/vis-updates
vitvakatu Nov 13, 2024
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
25 changes: 16 additions & 9 deletions app/gui/src/project-view/components/GraphEditor/GraphVisualization/visualizationData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export function useVisualizationData({

const defaultVisualizationRaw = projectStore.useVisualizationData(
configForGettingDefaultVisualization,
) as ShallowRef<Result<{ library: { name: string } | null; name: string } | undefined>>
).data as ShallowRef<Result<{ library: { name: string } | null; name: string } | undefined>>

const defaultVisualizationForCurrentNodeSource = computed<VisualizationIdentifier | undefined>(
() => {
Expand Down Expand Up @@ -108,14 +108,15 @@ export function useVisualizationData({
return false
})

const nodeVisualizationData = projectStore.useVisualizationData(() => {
const dataSourceValue = toValue(dataSource)
if (dataSourceValue?.type !== 'node') return
return {
...visPreprocessor.value,
expressionId: dataSourceValue.nodeId,
}
})
const { data: nodeVisualizationData, reconnect: reconnectVisualization } =
projectStore.useVisualizationData(() => {
const dataSourceValue = toValue(dataSource)
if (dataSourceValue?.type !== 'node') return
return {
...visPreprocessor.value,
expressionId: dataSourceValue.nodeId,
}
})

const expressionVisualizationData = computedAsync(
() => {
Expand Down Expand Up @@ -241,6 +242,12 @@ export function useVisualizationData({
}
return visualization.value
})
watch(effectiveVisualization, (_, oldVis) => {
// Reconnect when the visualization kind changes, but ignore transitions from the loading state.
if (oldVis !== LoadingVisualization) {
reconnectVisualization()
}
})

// Visualization-provided configuration
const toolbarOverlay = ref(false)
Expand Down
6 changes: 4 additions & 2 deletions .../src/project-view/components/GraphEditor/widgets/WidgetFunction/widgetFunctionCallInfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ export function useWidgetFunctionCallInfo(
getExpressionInfo(id: AstId): ExpressionInfo | undefined
},
project: {
useVisualizationData(config: Ref<Opt<NodeVisualizationConfiguration>>): Ref<Result<any> | null>
useVisualizationData(config: Ref<Opt<NodeVisualizationConfiguration>>): {
data: Ref<Result<any> | null>
}
},
) {
const methodCallInfo = computed(() => getMethodCallInfoRecursively(toValue(input).value, graphDb))
Expand Down Expand Up @@ -144,7 +146,7 @@ export function useWidgetFunctionCallInfo(
const visualizationData = project.useVisualizationData(visualizationConfig)

const widgetConfiguration = computed(() => {
const data = visualizationData.value
const data = visualizationData.data.value
if (data?.ok) {
const parseResult = argsWidgetConfigurationSchema.safeParse(data.value)
if (parseResult.success) {
Expand Down
16 changes: 11 additions & 5 deletions app/gui/src/project-view/stores/project/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,12 @@ export const { provideFn: provideProjectStore, injectFn: useProjectStore } = cre
})

function useVisualizationData(configuration: WatchSource<Opt<NodeVisualizationConfiguration>>) {
const id = random.uuidv4() as Uuid
const newId = () => random.uuidv4() as Uuid
const id = ref(newId())

watch(
configuration,
(config, _, onCleanup) => {
[configuration, id],
([config, id], _, onCleanup) => {
executionContext.setVisualization(id, config)
onCleanup(() => executionContext.setVisualization(id, null))
},
Expand All @@ -250,7 +251,12 @@ export const { provideFn: provideProjectStore, injectFn: useProjectStore } = cre
{ immediate: true, flush: 'post' },
)

return computed(() => parseVisualizationData(visualizationDataRegistry.getRawData(id)))
return {
data: computed(() =>
parseVisualizationData(visualizationDataRegistry.getRawData(id.value)),
),
reconnect: () => (id.value = newId()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we also automatically regenerate ID always when preprocessor (expression or visualizationModule) changed? I think this should be a "general" rule, not only for visualization displayed under graph.

It may be a bit tricky to avoid recursive watch run: my idea is to make current id ref to be a tuple of (id, config); the current watch just watches this tuple, we add additional watch of configuration which sets new tuple re-using or regenerating id, and reconnect just regenerates id while keeping config. But maybe it may be even simpler.

}
}

const dataflowErrors = new ReactiveMapping(computedValueRegistry.db, (id, info) => {
Expand All @@ -267,7 +273,7 @@ export const { provideFn: provideProjectStore, injectFn: useProjectStore } = cre
}
: null,
)
const data = useVisualizationData(config)
const { data } = useVisualizationData(config)
return computed<{ kind: 'Dataflow'; message: string } | undefined>(() => {
const visResult = data.value
if (!visResult) return
Expand Down
Loading