-
Notifications
You must be signed in to change notification settings - Fork 511
feat: add workflow execution state indicators to tabs #8592
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
Open
christian-byrne
wants to merge
12
commits into
main
Choose a base branch
from
feat/tab-execution-state-indicator
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
ed9a8a2
feat: add workflow execution state indicators to tabs
christian-byrne 19c28b3
fix: rename promptIdToWorkflowId/runningPromptIds to match jobId refa…
christian-byrne 2093a3f
fix: update test to use renamed storeJob and nodeProgressStatesByJob
christian-byrne 9850888
fix: align execution indicator icons to center horizontal axis of tab
christian-byrne c6ceadf
refactor: remove redundant seenWorkflows set in queueStore
christian-byrne 2bbeeef
refactor: rename WorkflowExecutionResult.promptId to jobId
christian-byrne d0e0902
fix: clear stale execution indicators for cancelled jobs
christian-byrne 9fe7f7e
fix: address CodeRabbit review feedback
christian-byrne c9fd8a2
fix: strip workflow id on duplicate to prevent inherited execution in…
christian-byrne a39a7d6
merge: resolve conflicts with main
christian-byrne 117f62f
[automated] Apply ESLint and Oxfmt fixes
actions-user f426af0
merge: resolve conflicts with main
christian-byrne File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next
Next commit
feat: add workflow execution state indicators to tabs
- Add WorkflowExecutionResult type and lastExecutionResultByWorkflowId to executionStore - Create useWorkflowExecutionState composable for tracking execution state - Create WorkflowExecutionIndicator.vue with running/completed/error icons - Integrate indicator into WorkflowTab.vue with 5s auto-clear for completed - Add WorkflowExecutionBadge.vue for sidebar display - Add i18n accessibility labels Amp-Thread-ID: https://ampcode.com/threads/T-019c2557-1ba9-726a-8e93-978864992fd4
- Loading branch information
commit ed9a8a2aae69fe62608ddf20cc380ea64adc8c44
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
20 changes: 20 additions & 0 deletions
20
src/components/sidebar/tabs/workflows/WorkflowExecutionBadge.vue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| <script setup lang="ts"> | ||
| import { computed } from 'vue' | ||
|
|
||
| import WorkflowExecutionIndicator from '@/components/topbar/WorkflowExecutionIndicator.vue' | ||
| import { useWorkflowExecutionState } from '@/composables/useWorkflowExecutionState' | ||
| import type { ComfyWorkflow } from '@/platform/workflow/management/stores/workflowStore' | ||
|
|
||
| const { workflow } = defineProps<{ workflow?: ComfyWorkflow }>() | ||
|
|
||
| const workflowId = computed(() => { | ||
| return workflow?.activeState?.id ?? workflow?.initialState?.id | ||
| }) | ||
|
|
||
| const { state } = useWorkflowExecutionState(workflowId) | ||
| const showIndicator = computed(() => state.value !== 'idle') | ||
| </script> | ||
|
|
||
| <template> | ||
| <WorkflowExecutionIndicator v-if="showIndicator" :state="state" /> | ||
| </template> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| import { mount } from '@vue/test-utils' | ||
| import { describe, expect, it } from 'vitest' | ||
| import { createI18n } from 'vue-i18n' | ||
|
|
||
| import type { WorkflowExecutionState } from '@/stores/executionStore' | ||
|
|
||
| import WorkflowExecutionIndicator from './WorkflowExecutionIndicator.vue' | ||
|
|
||
| const i18n = createI18n({ | ||
| legacy: false, | ||
| locale: 'en', | ||
| messages: { | ||
| en: { | ||
| workflowExecution: { | ||
| running: 'Workflow is running', | ||
| completed: 'Workflow completed successfully', | ||
| error: 'Workflow execution failed' | ||
| } | ||
| } | ||
| } | ||
| }) | ||
|
|
||
| describe('WorkflowExecutionIndicator', () => { | ||
| const mountWithI18n = (props: { state: WorkflowExecutionState }) => | ||
| mount(WorkflowExecutionIndicator, { | ||
| props, | ||
| global: { | ||
| plugins: [i18n] | ||
| } | ||
| }) | ||
|
|
||
| it('renders nothing for idle state', () => { | ||
| const wrapper = mountWithI18n({ state: 'idle' }) | ||
| expect(wrapper.find('i').exists()).toBe(false) | ||
| }) | ||
|
|
||
| it.each<{ state: WorkflowExecutionState; label: string }>([ | ||
| { state: 'running', label: 'Workflow is running' }, | ||
| { state: 'completed', label: 'Workflow completed successfully' }, | ||
| { state: 'error', label: 'Workflow execution failed' } | ||
| ])('renders accessible icon for $state state', ({ state, label }) => { | ||
| const wrapper = mountWithI18n({ state }) | ||
| const icon = wrapper.find('i') | ||
| expect(icon.exists()).toBe(true) | ||
| expect(icon.attributes('aria-label')).toBe(label) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| <template> | ||
| <i | ||
| v-if="state === 'running'" | ||
| class="icon-[lucide--loader-circle] size-4 shrink-0 animate-spin text-muted-foreground" | ||
| :aria-label="$t('workflowExecution.running')" | ||
| /> | ||
| <i | ||
| v-else-if="state === 'completed'" | ||
| class="icon-[lucide--circle-check] size-4 shrink-0 text-jade-600" | ||
| :aria-label="$t('workflowExecution.completed')" | ||
| /> | ||
| <i | ||
| v-else-if="state === 'error'" | ||
| class="icon-[lucide--circle-alert] size-4 shrink-0 text-coral-600" | ||
| :aria-label="$t('workflowExecution.error')" | ||
| /> | ||
| </template> | ||
|
|
||
| <script setup lang="ts"> | ||
| import type { WorkflowExecutionState } from '@/stores/executionStore' | ||
|
|
||
| const { state } = defineProps<{ | ||
| state: WorkflowExecutionState | ||
| }>() | ||
| </script> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import { createTestingPinia } from '@pinia/testing' | ||
| import { setActivePinia } from 'pinia' | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import { ref } from 'vue' | ||
|
|
||
| import type { WorkflowExecutionState } from '@/stores/executionStore' | ||
|
|
||
| import { useWorkflowExecutionState } from './useWorkflowExecutionState' | ||
|
|
||
| const _workflowExecutionStates = ref(new Map<string, WorkflowExecutionState>()) | ||
| const _clearWorkflowExecutionResult = vi.fn() | ||
|
|
||
| vi.mock('@/stores/executionStore', () => ({ | ||
| useExecutionStore: () => ({ | ||
| getWorkflowExecutionState: (wid: string | undefined) => { | ||
| if (!wid) return 'idle' | ||
| return _workflowExecutionStates.value.get(wid) ?? 'idle' | ||
| }, | ||
| clearWorkflowExecutionResult: _clearWorkflowExecutionResult | ||
| }) | ||
| })) | ||
|
|
||
| describe('useWorkflowExecutionState', () => { | ||
| beforeEach(() => { | ||
| setActivePinia(createTestingPinia({ stubActions: false })) | ||
| vi.clearAllMocks() | ||
| _workflowExecutionStates.value = new Map() | ||
| }) | ||
|
|
||
| it('returns idle when workflowId is undefined', () => { | ||
| const { state } = useWorkflowExecutionState(undefined) | ||
| expect(state.value).toBe('idle') | ||
| }) | ||
|
|
||
| it('returns idle when no execution data exists', () => { | ||
| const { state } = useWorkflowExecutionState('workflow-1') | ||
| expect(state.value).toBe('idle') | ||
| }) | ||
|
|
||
| it('returns state from execution store map', () => { | ||
| _workflowExecutionStates.value = new Map([['workflow-1', 'running']]) | ||
| const { state } = useWorkflowExecutionState('workflow-1') | ||
| expect(state.value).toBe('running') | ||
| }) | ||
|
|
||
| it('reacts to workflowId ref changes', () => { | ||
| const wfId = ref<string | undefined>('workflow-1') | ||
| _workflowExecutionStates.value = new Map([ | ||
| ['workflow-1', 'running'], | ||
| ['workflow-2', 'error'] | ||
| ]) | ||
|
|
||
| const { state } = useWorkflowExecutionState(wfId) | ||
| expect(state.value).toBe('running') | ||
|
|
||
| wfId.value = 'workflow-2' | ||
| expect(state.value).toBe('error') | ||
| }) | ||
|
|
||
| it('clearResult delegates to executionStore', () => { | ||
| const { clearResult } = useWorkflowExecutionState('workflow-1') | ||
| clearResult() | ||
| expect(_clearWorkflowExecutionResult).toHaveBeenCalledWith('workflow-1') | ||
| }) | ||
|
|
||
| it('clearResult does nothing when workflowId is undefined', () => { | ||
| const { clearResult } = useWorkflowExecutionState(undefined) | ||
| clearResult() | ||
| expect(_clearWorkflowExecutionResult).not.toHaveBeenCalled() | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import type { MaybeRefOrGetter } from 'vue' | ||
| import { computed, toValue } from 'vue' | ||
|
|
||
| import type { WorkflowExecutionState } from '@/stores/executionStore' | ||
| import { useExecutionStore } from '@/stores/executionStore' | ||
|
|
||
| export function useWorkflowExecutionState( | ||
| workflowId: MaybeRefOrGetter<string | undefined> | ||
| ) { | ||
| const executionStore = useExecutionStore() | ||
|
|
||
| const state = computed<WorkflowExecutionState>(() => | ||
| executionStore.getWorkflowExecutionState(toValue(workflowId)) | ||
| ) | ||
|
|
||
| function clearResult() { | ||
| const wid = toValue(workflowId) | ||
| if (wid) { | ||
| executionStore.clearWorkflowExecutionResult(wid) | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| state, | ||
| clearResult | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.