Merge pull request #456 from PrefectHQ/fix/artifacts-visibility-toggle

Bugfix: hide artifacts when setting changes

src/consts.ts

@@ -24,9 +24,9 @@ export const DEFAULT_ROOT_FLOW_STATE_Z_INDEX = 3
 export const DEFAULT_ROOT_ARTIFACT_Z_INDEX = 4
 
 export const DEFAULT_SUBFLOW_BORDER_Z_INDEX = 0
-export const DEFAULT_SUBFLOW_NODES_Z_INDEX = 1
-export const DEFAULT_SUBFLOW_EVENT_Z_INDEX = 2
-export const DEFAULT_SUBFLOW_STATE_Z_INDEX = 3
-export const DEFAULT_SUBFLOW_ARTIFACT_Z_INDEX = 4
-export const DEFAULT_SUBFLOW_NODE_Z_INDEX = 5
-export const DEFAULT_SUBFLOW_LABEL_Z_INDEX = 6
+export const DEFAULT_SUBFLOW_EVENT_Z_INDEX = 1
+export const DEFAULT_SUBFLOW_STATE_Z_INDEX = 2
+export const DEFAULT_SUBFLOW_NODES_Z_INDEX = 2
+export const DEFAULT_SUBFLOW_ARTIFACT_Z_INDEX = 3
+export const DEFAULT_SUBFLOW_NODE_Z_INDEX = 4
+export const DEFAULT_SUBFLOW_LABEL_Z_INDEX = 5

src/factories/flowRunArtifact.ts

@@ -105,9 +105,11 @@ export async function flowRunArtifactFactory<T extends ArtifactFactoryOptions>(o
 
     const x = scale(date) * viewport.scale._x + viewport.worldTransform.tx
     const centeredX = x - (element.width - selectedOffset) / 2
+
+    const bottomOffset = flowHasEvents && !settings.disableEvents ? eventTargetSize : flowStateSelectedBarHeight
     const y = application.screen.height
       - (element.height - selectedOffset)
-      - (flowHasEvents ? eventTargetSize : flowStateSelectedBarHeight)
+      - bottomOffset
 
     element.position.set(centeredX, y)
   }

src/factories/node.ts

@@ -73,10 +73,6 @@ export async function nodeContainerFactory(node: RunGraphNode) {
       createArtifacts(newNodeData.artifacts),
     ])
 
-    if (artifactsContainer) {
-      artifactsContainer.visible = layout.isTemporal() ? !settings.disableArtifacts : false
-    }
-
     if (newNodeData.end_time) {
       stopTicking()
     }
@@ -85,12 +81,14 @@ export async function nodeContainerFactory(node: RunGraphNode) {
   }
 
   async function createArtifacts(artifactsData?: RunGraphArtifact[]): Promise<void> {
-    if (!artifactsData || settings.disableArtifacts || !layout.isTemporal()) {
+    if (!artifactsData) {
       return
     }
 
-    if (!artifactsContainer) {
-      createArtifactsContainer()
+    createArtifactsContainer()
+
+    if (settings.disableArtifacts || !layout.isTemporal()) {
+      return
     }
 
     const promises: Promise<void>[] = []
@@ -119,8 +117,18 @@ export async function nodeContainerFactory(node: RunGraphNode) {
   }
 
   function createArtifactsContainer(): void {
-    artifactsContainer = new Container()
-    container.addChild(artifactsContainer)
+    if (layout.isTemporal() && !settings.disableArtifacts) {
+      if (!artifactsContainer) {
+        artifactsContainer = new Container()
+      }
+
+      container.addChild(artifactsContainer)
+      return
+    }
+
+    if (artifactsContainer) {
+      container.removeChild(artifactsContainer)
+    }
   }
 
   function alignArtifacts(): void {
@@ -179,6 +187,7 @@ export async function nodeContainerFactory(node: RunGraphNode) {
       layout.horizontal,
       layout.horizontalScaleMultiplier,
       config.styles.colorMode,
+      artifactCount > 0 && settings.disableArtifacts,
     ]
 
     return values.join('-')

src/factories/nodeFlowRun.ts

@@ -23,14 +23,15 @@ import { NodeSize } from '@/models/layout'
 import { RunGraphFetchEventsOptions, RunGraphNode } from '@/models/RunGraph'
 import { waitForConfig } from '@/objects/config'
 import { cull } from '@/objects/culling'
-import { layout } from '@/objects/settings'
+import { layout, waitForSettings } from '@/objects/settings'
 
 export type FlowRunContainer = Awaited<ReturnType<typeof flowRunContainerFactory>>
 
 // eslint-disable-next-line @typescript-eslint/explicit-function-return-type
 export async function flowRunContainerFactory(node: RunGraphNode) {
   const container = new BoundsContainer()
   const config = await waitForConfig()
+  const settings = await waitForSettings()
 
   const { element: bar, render: renderBar } = await nodeBarFactory()
   const { element: label, render: renderLabelText } = await nodeLabelFactory()
@@ -159,6 +160,13 @@ export async function flowRunContainerFactory(node: RunGraphNode) {
   }
 
   async function renderEvents(data?: RunGraphEvent[]): Promise<void> {
+    if (!isOpen || !layout.isTemporal() || settings.disableEvents) {
+      container.removeChild(nodesEvents)
+      return
+    }
+
+    container.addChild(nodesEvents)
+
     const { height } = getSize()
 
     nodesEvents.position = { x: 0, y: height - config.styles.eventBottomMargin }
@@ -172,10 +180,17 @@ export async function flowRunContainerFactory(node: RunGraphNode) {
   }
 
   async function renderArtifacts(data?: RunGraphArtifact[]): Promise<void> {
+    if (!isOpen || !layout.isTemporal() || settings.disableArtifacts) {
+      container.removeChild(nodesArtifacts)
+      return
+    }
+
+    container.addChild(nodesArtifacts)
+
     const { eventTargetSize, flowStateSelectedBarHeight } = config.styles
     const { height } = getSize()
 
-    const y = height - (hasEvents ? eventTargetSize : flowStateSelectedBarHeight)
+    const y = height - (hasEvents && !settings.disableEvents ? eventTargetSize : flowStateSelectedBarHeight)
 
     nodesArtifacts.position = { x: 0, y }
 
@@ -190,8 +205,6 @@ export async function flowRunContainerFactory(node: RunGraphNode) {
   async function open(): Promise<void> {
     isOpen = true
     container.addChild(nodesState)
-    container.addChild(nodesEvents)
-    container.addChild(nodesArtifacts)
     container.addChild(nodesContainer)
     container.addChild(border)
 
@@ -207,10 +220,10 @@ export async function flowRunContainerFactory(node: RunGraphNode) {
   async function close(): Promise<void> {
     isOpen = false
     container.removeChild(nodesState)
-    container.removeChild(nodesEvents)
-    container.removeChild(nodesArtifacts)
     container.removeChild(nodesContainer)
     container.removeChild(border)
+    container.removeChild(nodesEvents)
+    container.removeChild(nodesArtifacts)
     stopNodesWorker()
 
     await Promise.all([
@@ -282,8 +295,11 @@ export async function flowRunContainerFactory(node: RunGraphNode) {
       artifactIconSize,
     } = config.styles
 
-    const artifactsHeight = hasArtifacts ? artifactIconSize + artifactPaddingY * 2 : 0
-    const eventsHeight = hasEvents ? eventTargetSize + eventBottomMargin : 0
+    const showArtifacts = hasArtifacts && layout.isTemporal() && !settings.disableArtifacts
+    const artifactsHeight = showArtifacts ? artifactIconSize + artifactPaddingY * 2 : 0
+
+    const showEvents = hasEvents && layout.isTemporal() && !settings.disableEvents
+    const eventsHeight = showEvents ? eventTargetSize + eventBottomMargin : 0
 
     const nodesHeight = isOpen
       ? nodes.height + artifactsHeight + eventsHeight + nodesPadding * 2

src/factories/runArtifacts.ts

@@ -6,7 +6,6 @@ import { ArtifactClusterFactory } from '@/factories/artifactCluster'
 import { flowRunArtifactFactory } from '@/factories/flowRunArtifact'
 import { nodeFlowRunArtifactFactory } from '@/factories/nodeFlowRunArtifact'
 import { RunGraphArtifact } from '@/models'
-import { emitter } from '@/objects/events'
 import { layout, waitForSettings } from '@/objects/settings'
 import { clusterHorizontalCollisions } from '@/utilities/detectHorizontalCollisions'
 
@@ -27,19 +26,7 @@ export async function runArtifactsFactory({ isRoot, parentStartDate }: RunEvents
   const container = new Container()
   let internalData: RunGraphArtifact[] | null = null
 
-  emitter.on('layoutSettingsUpdated', () => render())
-
   async function render(newData?: RunGraphArtifact[]): Promise<void> {
-    if (!layout.isTemporal()) {
-      container.visible = false
-    } else {
-      container.visible = !settings.disableEvents
-    }
-
-    if (settings.disableArtifacts || !layout.isTemporal()) {
-      return
-    }
-
     if (newData) {
       internalData = newData
     }

src/factories/runEvents.ts

@@ -27,20 +27,9 @@ export async function runEventsFactory({ isRoot, parentStartDate }: RunEventsFac
   const container = new Container()
   let internalData: RunGraphEvent[] | null = null
 
-  emitter.on('layoutSettingsUpdated', () => render())
   emitter.on('scaleUpdated', () => update())
 
   async function render(newData?: RunGraphEvent[]): Promise<void> {
-    if (!layout.isTemporal()) {
-      container.visible = false
-    } else {
-      container.visible = !settings.disableEvents
-    }
-
-    if (settings.disableEvents || !layout.isTemporal()) {
-      return
-    }
-
     if (newData) {
       internalData = newData
     }

src/objects/flowRunArtifacts.ts

@@ -4,17 +4,24 @@ import { RunGraphData } from '@/models/RunGraph'
 import { waitForApplication } from '@/objects/application'
 import { emitter } from '@/objects/events'
 import { waitForRunData } from '@/objects/nodes'
+import { layout, waitForSettings } from '@/objects/settings'
 
 export async function startFlowRunArtifacts(): Promise<void> {
   const application = await waitForApplication()
   const data = await waitForRunData()
+  const settings = await waitForSettings()
   const { element, render: renderArtifacts, update } = await runArtifactsFactory({ isRoot: true })
 
   element.zIndex = DEFAULT_ROOT_ARTIFACT_Z_INDEX
-  application.stage.addChild(element)
 
   function render(newData?: RunGraphData): void {
-    renderArtifacts(newData?.artifacts)
+    if (layout.isTemporal() && !settings.disableArtifacts) {
+      application.stage.addChild(element)
+      renderArtifacts(newData?.artifacts)
+      return
+    }
+
+    application.stage.removeChild(element)
   }
 
   if (data.artifacts) {
@@ -26,6 +33,7 @@ export async function startFlowRunArtifacts(): Promise<void> {
   emitter.on('runDataCreated', (data) => render(data))
   emitter.on('runDataUpdated', (data) => render(data))
   emitter.on('configUpdated', () => render())
+  emitter.on('layoutSettingsUpdated', () => render())
 }
 
 export function stopFlowRunArtifacts(): void {

src/objects/flowRunEvents.ts

@@ -5,36 +5,44 @@ import { RunGraphEvent } from '@/models'
 import { waitForApplication } from '@/objects/application'
 import { waitForConfig } from '@/objects/config'
 import { EventKey, emitter, waitForEvent } from '@/objects/events'
-import { waitForSettings } from '@/objects/settings'
+import { layout, waitForSettings } from '@/objects/settings'
 
 let stopEventData: (() => void) | null = null
 let rootGraphEvents: RunGraphEvent[] | null = null
 
 export async function startFlowRunEvents(): Promise<void> {
   const application = await waitForApplication()
   const config = await waitForConfig()
+  const settings = await waitForSettings()
 
-  const { element, render, update } = await runEventsFactory({ isRoot: true })
+  const { element, render: renderEvents, update } = await runEventsFactory({ isRoot: true })
 
   element.zIndex = DEFAULT_ROOT_EVENT_Z_INDEX
-  application.stage.addChild(element)
 
-  const response = await eventDataFactory(config.runId, async data => {
+  async function render(data?: RunGraphEvent[]): Promise<void> {
+    if (!layout.isTemporal() || settings.disableEvents) {
+      application.stage.removeChild(element)
+      return
+    }
+
+    application.stage.addChild(element)
+
+    await renderEvents(data)
+  }
+
+  const response = await eventDataFactory(config.runId, data => {
     const event: EventKey = rootGraphEvents ? 'eventDataUpdated' : 'eventDataCreated'
 
     rootGraphEvents = data
 
     emitter.emit(event, rootGraphEvents)
 
-    // this makes sure the layout settings are initialized prior to rendering
-    // important to prevent double rendering on the first render
-    await waitForSettings()
-
     render(data)
   })
 
   emitter.on('configUpdated', () => render())
   emitter.on('viewportMoved', () => update())
+  emitter.on('layoutSettingsUpdated', () => render())
 
   stopEventData = response.stop