Skip to content

[WIP] Fix: Escape closes Settings dialog if login dialog open #3996 #4299

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

Closed
wants to merge 39 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
770ecea
add close button on login dialog
ssidharth010 Jun 29, 2025
d6a8f98
[docs] add component communication best practices (#4302)
christian-byrne Jun 30, 2025
eb8b67d
[docs] Update nested README files with comprehensive component listin…
christian-byrne Jun 30, 2025
5bbed91
usage log table (#4288)
jtydhr88 Jun 30, 2025
fada8bf
Follow-up on #4256 (#4307)
webfiltered Jun 30, 2025
64a2a5b
[fix] Mock release API in browser tests to prevent UI interference (#…
christian-byrne Jul 1, 2025
c75015c
Fix helper menu issues and align with the design. (#4261)
comfyui-wiki Jul 1, 2025
6470a0b
[CodeHealth] Follow-up on #4288 - code style / async (#4308)
webfiltered Jul 1, 2025
bf3dcc8
[chore] Update litegraph to 0.16.2 (#4315)
comfy-pr-bot Jul 1, 2025
d92c282
[chore] Update litegraph to 0.16.3 (#4316)
comfy-pr-bot Jul 1, 2025
26c106c
Allow prerelease using version bump action (#4318)
webfiltered Jul 1, 2025
8d63600
Use prerelease flag for draft releases (#4319)
webfiltered Jul 1, 2025
df71094
[CI] Skip i18n in unrelated PRs (#4320)
webfiltered Jul 1, 2025
d68391a
[CI] Fix prerelease version tag not set (#4322)
webfiltered Jul 1, 2025
e6f90e3
1.24.0-0 (#4321)
comfy-pr-bot Jul 1, 2025
8f825c0
[docs] add code quality guidelines for i18n, async cleanup, and error…
christian-byrne Jul 2, 2025
f57f97c
[TS] Remove frontend-only typing from litegraph (#4325)
webfiltered Jul 2, 2025
35ff882
[3d] better solution to support reading extra resource/texture (#4209)
jtydhr88 Jul 2, 2025
959ab3b
[feat] Add ESLint i18n enforcement and fix hardcoded strings (#4327)
christian-byrne Jul 2, 2025
5cc1a8d
[test] Add release notification browser tests (#4311)
christian-byrne Jul 2, 2025
a457534
[System Pop Up] Improve help center menu behavior and Electron compat…
bmcomfy Jul 2, 2025
f290c00
[chore] Update litegraph to 0.16.4 (#4335)
comfy-pr-bot Jul 3, 2025
a51c098
1.24.0-1 (#4336)
comfy-pr-bot Jul 3, 2025
7befec5
Add unused i18n keys detection to pre-commit hook (#4328)
christian-byrne Jul 3, 2025
1b4ad61
[chore] Update Comfy Registry API types from comfy-api@4b0dc99 (#4340)
comfy-pr-bot Jul 3, 2025
44bbfa9
[feat] Implement getNodeByComfyNodeName API integration (#4343)
christian-byrne Jul 4, 2025
ee50885
Vue expose (#4265)
jtydhr88 Jul 4, 2025
3b435e3
[fix] Add dynamic pricing for Ideogram nodes based on num_images para…
christian-byrne Jul 4, 2025
c1db367
add installedVersion (#4337)
jtydhr88 Jul 4, 2025
8f4e807
[fix] move i18n pre-commit check inside Windows conditional block (#4…
christian-byrne Jul 5, 2025
92b65ca
[fix] Remove optional designation from issue report details field (#4…
christian-byrne Jul 5, 2025
35556eb
fIx: side toolbar tab tooltip not reactive when changing locale (#4213)
hyq2015 Jul 5, 2025
0c4339f
[TS] Update callbacks using CanvasMouseEvent #1104 (#4358)
webfiltered Jul 5, 2025
191b457
[fix] Add dynamic pricing for API nodes with quantity parameters (#4362)
christian-byrne Jul 6, 2025
282c866
fix: handle close on esc based on the top dialog
ssidharth010 Jul 6, 2025
7241255
add close button on login dialog
ssidharth010 Jun 29, 2025
5a3c084
fix: handle close on esc based on the top dialog
ssidharth010 Jul 6, 2025
ce94448
Merge branch 'fix/dialog-close' of https://github.com/ssidharth010/Co…
ssidharth010 Jul 6, 2025
be3ca28
fix: track active dialog
ssidharth010 Jul 6, 2025
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
6 changes: 6 additions & 0 deletions .github/workflows/i18n.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ name: Update Locales
on:
pull_request:
branches: [ main, master, dev* ]
paths-ignore:
- '.github/**'
- '.husky/**'
- '.vscode/**'
- 'browser_tests/**'
- 'tests-ui/**'

jobs:
update-locales:
Expand Down
16 changes: 13 additions & 3 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ jobs:
contains(github.event.pull_request.labels.*.name, 'Release')
outputs:
version: ${{ steps.current_version.outputs.version }}
is_prerelease: ${{ steps.check_prerelease.outputs.is_prerelease }}
steps:
- name: Checkout code
uses: actions/checkout@v4
Expand All @@ -24,6 +25,15 @@ jobs:
- name: Get current version
id: current_version
run: echo "version=$(node -p "require('./package.json').version")" >> $GITHUB_OUTPUT
- name: Check if prerelease
id: check_prerelease
run: |
VERSION=${{ steps.current_version.outputs.version }}
if [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+- ]]; then
echo "is_prerelease=true" >> $GITHUB_OUTPUT
else
echo "is_prerelease=false" >> $GITHUB_OUTPUT
fi
- name: Build project
env:
SENTRY_DSN: ${{ secrets.SENTRY_DSN }}
Expand Down Expand Up @@ -62,9 +72,9 @@ jobs:
dist.zip
tag_name: v${{ needs.build.outputs.version }}
target_commitish: ${{ github.event.pull_request.base.ref }}
make_latest: ${{ github.event.pull_request.base.ref == 'main' }}
draft: ${{ github.event.pull_request.base.ref != 'main' }}
prerelease: false
make_latest: ${{ github.event.pull_request.base.ref == 'main' && needs.build.outputs.is_prerelease == 'false' }}
draft: ${{ github.event.pull_request.base.ref != 'main' || needs.build.outputs.is_prerelease == 'true' }}
prerelease: ${{ needs.build.outputs.is_prerelease == 'true' }}
generate_release_notes: true

publish_pypi:
Expand Down
26 changes: 17 additions & 9 deletions .github/workflows/version-bump.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ on:
required: true
default: 'patch'
type: 'choice'
options:
- patch
- minor
- major
options: [patch, minor, major, prepatch, preminor, premajor, prerelease]
pre_release:
description: Pre-release ID (suffix)
required: false
default: ''
type: string

jobs:
bump-version:
Expand All @@ -33,19 +35,25 @@ jobs:
- name: Bump version
id: bump-version
run: |
npm version ${{ github.event.inputs.version_type }} --no-git-tag-version
npm version ${{ github.event.inputs.version_type }} --preid ${{ github.event.inputs.pre_release }} --no-git-tag-version
NEW_VERSION=$(node -p "require('./package.json').version")
echo "NEW_VERSION=$NEW_VERSION" >> $GITHUB_OUTPUT

- name: Format PR string
id: capitalised
run: |
CAPITALISED_TYPE=${{ github.event.inputs.version_type }}
echo "capitalised=${CAPITALISED_TYPE@u}" >> $GITHUB_OUTPUT

- name: Create Pull Request
uses: peter-evans/create-pull-request@271a8d0340265f705b14b6d32b9829c1cb33d45e
with:
token: ${{ secrets.PR_GH_TOKEN }}
commit-message: '[release] Bump version to ${{ steps.bump-version.outputs.NEW_VERSION }}'
title: '${{ steps.bump-version.outputs.NEW_VERSION }}'
commit-message: '[release] Increment version to ${{ steps.bump-version.outputs.NEW_VERSION }}'
title: ${{ steps.bump-version.outputs.NEW_VERSION }}
body: |
Automated version bump to ${{ steps.bump-version.outputs.NEW_VERSION }}
${{ steps.capitalised.outputs.capitalised }} version increment to ${{ steps.bump-version.outputs.NEW_VERSION }}
branch: version-bump-${{ steps.bump-version.outputs.NEW_VERSION }}
base: main
labels: |
Release
Release
4 changes: 4 additions & 0 deletions .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
if [[ "$OS" == "Windows_NT" ]]; then
npx.cmd lint-staged
# Check for unused i18n keys in staged files
npx.cmd tsx scripts/check-unused-i18n-keys.ts
else
npx lint-staged
# Check for unused i18n keys in staged files
npx tsx scripts/check-unused-i18n-keys.ts
fi
14 changes: 8 additions & 6 deletions CLAUDE.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
- use npm run to see what commands are available
- use `npm run` to see what commands are available
- For component communication, prefer Vue's event-based pattern (emit/@event-name) for state changes and notifications; use defineExpose with refs only for imperative operations that need direct control (like form.validate(), modal.open(), or editor.focus()); events promote loose coupling and are better for reusable components, while exposed methods are acceptable for tightly-coupled component pairs or when wrapping third-party libraries that require imperative APIs
- After making code changes, follow this general process: (1) Create unit tests, component tests, browser tests (if appropriate for each), (2) run unit tests, component tests, and browser tests until passing, (3) run typecheck, lint, format (with prettier) -- you can use `npm run` command to see the scripts available, (4) check if any READMEs (including nested) or documentation needs to be updated, (5) Decide whether the changes are worth adding new content to the external documentation for (or would requires changes to the external documentation) at https://docs.comfy.org, then present your suggestion
- When referencing PrimeVue, you can get all the docs here: https://primevue.org. Do this instead of making up or inferring names of Components
- When trying to set tailwind classes for dark theme, use "dark-theme:" prefix rather than "dark:"
- Never add lines to PR descriptions that say "Generated with Claude Code"
- Never add lines to PR descriptions or commit messages that say "Generated with Claude Code"
- When making PR names and commit messages, if you are going to add a prefix like "docs:", "feat:", "bugfix:", use square brackets around the prefix term and do not use a colon (e.g., should be "[docs]" rather than "docs:").
- When I reference GitHub Repos related to Comfy-Org, you should proactively fetch or read the associated information in the repo. To do so, you should exhaust all options: (1) Check if we have a local copy of the repo, (2) Use the GitHub API to fetch the information; you may want to do this IN ADDITION to the other options, especially for reading specific branches/PRs/comments/reviews/metadata, and (3) curl the GitHub website and parse the html or json responses
- For information about ComfyUI, ComfyUI_frontend, or ComfyUI-Manager, you can web search or download these wikis: https://deepwiki.com/Comfy-Org/ComfyUI-Manager, https://deepwiki.com/Comfy-Org/ComfyUI_frontend/1-overview, https://deepwiki.com/comfyanonymous/ComfyUI/2-core-architecture
Expand All @@ -17,7 +18,6 @@
- Use the Vue 3 Composition API instead of the Options API when writing Vue components. An exception is when overriding or extending a PrimeVue component for compatibility, you may use the Options API.
- when we are solving an issue we know the link/number for, we should add "Fixes #n" (where n is the issue number) to the PR description.
- Never write css if you can accomplish the same thing with tailwind utility classes
- Use setup() function for component logic
- Utilize ref and reactive for reactive state
- Implement computed properties with computed()
- Use watch and watchEffect for side effects
Expand All @@ -27,14 +27,12 @@
- Use Tailwind CSS for styling
- Leverage VueUse functions for performance-enhancing styles
- Use lodash for utility functions
- Use TypeScript for type safety
- Implement proper props and emits definitions
- Utilize Vue 3's Teleport component when needed
- Use Suspense for async components
- Implement proper error handling
- Follow Vue 3 style guide and naming conventions
- Use Vite for fast development and building
- Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json.
- IMPORTANT: Use vue-i18n for ALL user-facing strings - no hard-coded text in services/utilities. Place new translation entries in src/locales/en/main.json
- Avoid using `@ts-expect-error` to work around type issues. We needed to employ it to migrate to TypeScript, but it should not be viewed as an accepted practice or standard.
- DO NOT use deprecated PrimeVue components. Use these replacements instead:
* `Dropdown` → Use `Select` (import from 'primevue/select')
Expand All @@ -54,3 +52,7 @@
- Templates: `api.fileURL('/templates/default.json')`
- Extensions: `api.fileURL(extensionPath)` for loading JS modules
- Any static assets that exist in the public directory
- When implementing code that outputs raw HTML (e.g., using v-html directive), always ensure dynamic content has been properly sanitized with DOMPurify or validated through trusted sources. Prefer Vue templates over v-html when possible.
- For any async operations (API calls, timers, etc), implement cleanup/cancellation in component unmount to prevent memory leaks
- Extract complex template conditionals into separate components or computed properties
- Error messages should be actionable and user-friendly (e.g., "Failed to load data. Please refresh the page." instead of "Unknown error")
10 changes: 10 additions & 0 deletions browser_tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ A template with helpful information can be found in `.env_example`.
### Multiple Tests
If you are running Playwright tests in parallel or running the same test multiple times, the flag `--multi-user` must be added to the main ComfyUI process.

### Release API Mocking
By default, all tests mock the release API (`api.comfy.org/releases`) to prevent release notification popups from interfering with test execution. This is necessary because the release notifications can appear over UI elements and block test interactions.

To test with real release data, you can disable mocking:
```typescript
await comfyPage.setup({ mockReleases: false });
```

For tests that specifically need to test release functionality, see the example in `tests/releaseNotifications.spec.ts`.

## Running Tests

There are multiple ways to run the tests:
Expand Down
30 changes: 28 additions & 2 deletions browser_tests/fixtures/ComfyPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,34 @@ export class ComfyPage {
return this._history
}

async setup({ clearStorage = true }: { clearStorage?: boolean } = {}) {
async setup({
clearStorage = true,
mockReleases = true
}: {
clearStorage?: boolean
mockReleases?: boolean
} = {}) {
await this.goto()

// Mock release endpoint to prevent changelog popups
if (mockReleases) {
await this.page.route('**/releases**', async (route) => {
const url = route.request().url()
if (
url.includes('api.comfy.org') ||
url.includes('stagingapi.comfy.org')
) {
await route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify([])
})
} else {
await route.continue()
}
})
}

if (clearStorage) {
await this.page.evaluate((id) => {
localStorage.clear()
Expand Down Expand Up @@ -1086,7 +1112,7 @@ export const comfyPageFixture = base.extend<{
},
comfyMouse: async ({ comfyPage }, use) => {
const comfyMouse = new ComfyMouse(comfyPage)
use(comfyMouse)
await use(comfyMouse)
}
})

Expand Down
133 changes: 133 additions & 0 deletions browser_tests/tests/releaseNotifications.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import { expect } from '@playwright/test'

import { comfyPageFixture as test } from '../fixtures/ComfyPage'

test.describe('Release Notifications', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
})

test('should show help center with release information', async ({
comfyPage
}) => {
// Mock release API with test data instead of empty array
await comfyPage.page.route('**/releases**', async (route) => {
const url = route.request().url()
if (
url.includes('api.comfy.org') ||
url.includes('stagingapi.comfy.org')
) {
await route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify([
{
id: 1,
project: 'comfyui',
version: 'v0.3.44',
attention: 'medium',
content:
'## New Features\n\n- Added awesome feature\n- Fixed important bug',
published_at: new Date().toISOString()
}
])
})
} else {
await route.continue()
}
})

// Setup with release mocking disabled for this test
await comfyPage.setup({ mockReleases: false })

// Open help center
const helpCenterButton = comfyPage.page.locator('.comfy-help-center-btn')
await helpCenterButton.waitFor({ state: 'visible' })
await helpCenterButton.click()

// Verify help center menu appears
const helpMenu = comfyPage.page.locator('.help-center-menu')
await expect(helpMenu).toBeVisible()

// Verify "What's New?" section shows the release
const whatsNewSection = comfyPage.page.locator('.whats-new-section')
await expect(whatsNewSection).toBeVisible()

// Should show the release version
await expect(
whatsNewSection.locator('text=Comfy v0.3.44 Release')
).toBeVisible()

// Close help center by dismissable mask
await comfyPage.page.click('.help-center-backdrop')
await expect(helpMenu).not.toBeVisible()
})

test('should not show release notifications when mocked (default behavior)', async ({
comfyPage
}) => {
// Use default setup (mockReleases: true)
await comfyPage.setup()

// Open help center
const helpCenterButton = comfyPage.page.locator('.comfy-help-center-btn')
await helpCenterButton.waitFor({ state: 'visible' })
await helpCenterButton.click()

// Verify help center menu appears
const helpMenu = comfyPage.page.locator('.help-center-menu')
await expect(helpMenu).toBeVisible()

// Verify "What's New?" section shows no releases
const whatsNewSection = comfyPage.page.locator('.whats-new-section')
await expect(whatsNewSection).toBeVisible()

// Should show "No recent releases" message
await expect(
whatsNewSection.locator('text=No recent releases')
).toBeVisible()

// Should not show any popups or toasts
await expect(comfyPage.page.locator('.whats-new-popup')).not.toBeVisible()
await expect(
comfyPage.page.locator('.release-notification-toast')
).not.toBeVisible()
})

test('should handle release API errors gracefully', async ({ comfyPage }) => {
// Mock API to return an error
await comfyPage.page.route('**/releases**', async (route) => {
const url = route.request().url()
if (
url.includes('api.comfy.org') ||
url.includes('stagingapi.comfy.org')
) {
await route.fulfill({
status: 500,
contentType: 'application/json',
body: JSON.stringify({ error: 'Server error' })
})
} else {
await route.continue()
}
})

// Setup with release mocking disabled
await comfyPage.setup({ mockReleases: false })

// Open help center
const helpCenterButton = comfyPage.page.locator('.comfy-help-center-btn')
await helpCenterButton.waitFor({ state: 'visible' })
await helpCenterButton.click()

// Verify help center still works despite API error
const helpMenu = comfyPage.page.locator('.help-center-menu')
await expect(helpMenu).toBeVisible()

// Should show no releases due to error
const whatsNewSection = comfyPage.page.locator('.whats-new-section')
await expect(
whatsNewSection.locator('text=No recent releases')
).toBeVisible()
})
})
Loading