Conversation
duckcomfy
pushed a commit
to duckcomfy/ComfyUI_frontend
that referenced
this pull request
Jul 10, 2025
fix: show correct preview after painting an image in MaskEditor improve layer-selecting UX fix rectangular brushes add keyboard shortcuts to change brush size
benceruleanlu
pushed a commit
that referenced
this pull request
Aug 3, 2025
viva-jinyi
added a commit
that referenced
this pull request
Nov 17, 2025
Fixed issue where partial deletion failures were not reported to users.
Previously, if some assets failed to delete, the UI would still show
"all assets deleted successfully" which could mislead users about data loss.
Changes:
- Replaced Promise.all with Promise.allSettled in deleteMultipleAssets
- Track individual success/failure for each asset deletion
- Show different toast messages based on results:
- All success: success toast with count
- All failed: error toast
- Partial success: warning toast with succeeded/failed counts
- Improved error logging with asset names for debugging
Added i18n:
- mediaAsset.selection.partialDeleteSuccess: "{succeeded} deleted successfully, {failed} failed"
This addresses PR review comment #2 (Medium Priority).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
viva-jinyi
added a commit
that referenced
this pull request
Nov 17, 2025
Fixed issue where partial deletion failures were not reported to users.
Previously, if some assets failed to delete, the UI would still show
"all assets deleted successfully" which could mislead users about data loss.
Changes:
- Replaced Promise.all with Promise.allSettled in deleteMultipleAssets
- Track individual success/failure for each asset deletion
- Show different toast messages based on results:
- All success: success toast with count
- All failed: error toast
- Partial success: warning toast with succeeded/failed counts
- Improved error logging with asset names for debugging
Added i18n:
- mediaAsset.selection.partialDeleteSuccess: "{succeeded} deleted successfully, {failed} failed"
This addresses PR review comment #2 (Medium Priority).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
3 tasks
artokun
added a commit
to artokun/ComfyUI_frontend
that referenced
this pull request
Mar 20, 2026
…ectness - Cache bound subgraph slot lookup with version-based invalidation (#1, Comfy-Org#10) - Replace O(N) per-frame scan in drawConnections with _widgetSlotsDirty flag (Comfy-Org#5, Comfy-Org#11) - Remove dead composite-key fallback branch in findBoundSubgraphSlot (Comfy-Org#2) - Use isPromotedWidgetView() type guard instead of manual duck-typing (Comfy-Org#3) - Make label setter functional, propagating to bound subgraph slot (Comfy-Org#4) - Simplify get name() to use identityName ?? sourceWidgetName (Comfy-Org#6) - Import ISubgraphInput type instead of inline cast in widgetUtil (Comfy-Org#7) - Add tooltip field to SafeWidgetData interface (Comfy-Org#9) - Clear stale input.pos in removeWidget() and mark slots dirty (Comfy-Org#12)
artokun
added a commit
to artokun/ComfyUI_frontend
that referenced
this pull request
Mar 23, 2026
…ectness - Cache bound subgraph slot lookup with version-based invalidation (#1, Comfy-Org#10) - Replace O(N) per-frame scan in drawConnections with _widgetSlotsDirty flag (Comfy-Org#5, Comfy-Org#11) - Remove dead composite-key fallback branch in findBoundSubgraphSlot (Comfy-Org#2) - Use isPromotedWidgetView() type guard instead of manual duck-typing (Comfy-Org#3) - Make label setter functional, propagating to bound subgraph slot (Comfy-Org#4) - Simplify get name() to use identityName ?? sourceWidgetName (Comfy-Org#6) - Import ISubgraphInput type instead of inline cast in widgetUtil (Comfy-Org#7) - Add tooltip field to SafeWidgetData interface (Comfy-Org#9) - Clear stale input.pos in removeWidget() and mark slots dirty (Comfy-Org#12)
artokun
added a commit
to artokun/ComfyUI_frontend
that referenced
this pull request
Mar 23, 2026
…ectness - Cache bound subgraph slot lookup with version-based invalidation (#1, Comfy-Org#10) - Replace O(N) per-frame scan in drawConnections with _widgetSlotsDirty flag (Comfy-Org#5, Comfy-Org#11) - Remove dead composite-key fallback branch in findBoundSubgraphSlot (Comfy-Org#2) - Use isPromotedWidgetView() type guard instead of manual duck-typing (Comfy-Org#3) - Make label setter functional, propagating to bound subgraph slot (Comfy-Org#4) - Simplify get name() to use identityName ?? sourceWidgetName (Comfy-Org#6) - Import ISubgraphInput type instead of inline cast in widgetUtil (Comfy-Org#7) - Add tooltip field to SafeWidgetData interface (Comfy-Org#9) - Clear stale input.pos in removeWidget() and mark slots dirty (Comfy-Org#12)
artokun
added a commit
to artokun/ComfyUI_frontend
that referenced
this pull request
Mar 23, 2026
…ectness - Cache bound subgraph slot lookup with version-based invalidation (#1, Comfy-Org#10) - Replace O(N) per-frame scan in drawConnections with _widgetSlotsDirty flag (Comfy-Org#5, Comfy-Org#11) - Remove dead composite-key fallback branch in findBoundSubgraphSlot (Comfy-Org#2) - Use isPromotedWidgetView() type guard instead of manual duck-typing (Comfy-Org#3) - Make label setter functional, propagating to bound subgraph slot (Comfy-Org#4) - Simplify get name() to use identityName ?? sourceWidgetName (Comfy-Org#6) - Import ISubgraphInput type instead of inline cast in widgetUtil (Comfy-Org#7) - Add tooltip field to SafeWidgetData interface (Comfy-Org#9) - Clear stale input.pos in removeWidget() and mark slots dirty (Comfy-Org#12)
artokun
added a commit
to artokun/ComfyUI_frontend
that referenced
this pull request
Mar 23, 2026
…ectness - Cache bound subgraph slot lookup with version-based invalidation (#1, Comfy-Org#10) - Replace O(N) per-frame scan in drawConnections with _widgetSlotsDirty flag (Comfy-Org#5, Comfy-Org#11) - Remove dead composite-key fallback branch in findBoundSubgraphSlot (Comfy-Org#2) - Use isPromotedWidgetView() type guard instead of manual duck-typing (Comfy-Org#3) - Make label setter functional, propagating to bound subgraph slot (Comfy-Org#4) - Simplify get name() to use identityName ?? sourceWidgetName (Comfy-Org#6) - Import ISubgraphInput type instead of inline cast in widgetUtil (Comfy-Org#7) - Add tooltip field to SafeWidgetData interface (Comfy-Org#9) - Clear stale input.pos in removeWidget() and mark slots dirty (Comfy-Org#12)
artokun
added a commit
to artokun/ComfyUI_frontend
that referenced
this pull request
Mar 23, 2026
…ectness - Cache bound subgraph slot lookup with version-based invalidation (#1, Comfy-Org#10) - Replace O(N) per-frame scan in drawConnections with _widgetSlotsDirty flag (Comfy-Org#5, Comfy-Org#11) - Remove dead composite-key fallback branch in findBoundSubgraphSlot (Comfy-Org#2) - Use isPromotedWidgetView() type guard instead of manual duck-typing (Comfy-Org#3) - Make label setter functional, propagating to bound subgraph slot (Comfy-Org#4) - Simplify get name() to use identityName ?? sourceWidgetName (Comfy-Org#6) - Import ISubgraphInput type instead of inline cast in widgetUtil (Comfy-Org#7) - Add tooltip field to SafeWidgetData interface (Comfy-Org#9) - Clear stale input.pos in removeWidget() and mark slots dirty (Comfy-Org#12)
jtydhr88
pushed a commit
that referenced
this pull request
Apr 15, 2026
## Summary A follow-up PR of #11196. | # | Nit | Action | Reason | | :--- | :--- | :--- | :--- | | 1 | Replace `page.on('pageerror')` with request-wait | **Left as-is** | The `pageErrors` array is an accumulator checked at the end via `expect(pageErrors).toHaveLength(0)` – the goal is to assert that broken image URLs don't surface as uncaught JS exceptions during the test run. A request-wait can't substitute for that behavioral assertion, so the listener pattern is intentional here. | | 2 | Move helpers to a `vueNodes.getImageCompareHelper()` subclass | **Left as-is** | Helpers such as `setImageCompareValue` and `moveToPercentage` are only used in this file, making local encapsulation enough. Extracting them to a page object would increase the file/interface surface area and violate YAGNI; additionally, `AGENTS.md` clearly states to "minimize the exported values of each module. | | 3 | Use `TestIds` enum for test ID strings | **Fixed** – added `imageCompare` section to `TestIds` in `selectors.ts`; replaced all 8 inline string IDs in `imageCompare.spec.ts` with `TestIds.imageCompare.*` references | The project already has a `TestIds` convention for centralizing test IDs. Inline strings create drift risk between the Vue component and the test file. | | 4 | Move `expect.poll` bounding box check to helper/page object | **Left as-is** | This logic already lives inside `moveToPercentage`, which is a local helper. Moving it further to a page object is the same refactor as #2 above. | | 5 | Remove `// ---` style section header comments | **Fixed** – removed all 8 divider blocks from `imageCompare.spec.ts` | Consistent with project guidelines and your explicit preference. Test names already describe what each block does. | | 6 | Name magic numbers `400` and `350` | **Fixed** – introduced `minWidth = 400` and `minHeight = 350` constants in the test | Descriptive names make the constraint self-documenting and easier to update if the workflow asset changes. | <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: changes are limited to Playwright E2E test code and shared selector constants, with no production logic impacted. > > **Overview** > **E2E Image Compare tests now use centralized selectors.** Adds an `imageCompare` section to `TestIds` and updates `imageCompare.spec.ts` to reference `TestIds.imageCompare.*` instead of inline `data-testid` strings. > > Cleans up the spec by removing divider comments and naming the minimum size magic numbers (`minWidth`, `minHeight`) used in the node sizing assertion. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ece25be. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11260-test-address-review-nits-for-image-compare-E2E-3436d73d365081a69cacc1fff390035a) by [Unito](https://www.unito.io)
snomiao
pushed a commit
that referenced
this pull request
Apr 22, 2026
## Summary A follow-up PR of #11196. | # | Nit | Action | Reason | | :--- | :--- | :--- | :--- | | 1 | Replace `page.on('pageerror')` with request-wait | **Left as-is** | The `pageErrors` array is an accumulator checked at the end via `expect(pageErrors).toHaveLength(0)` – the goal is to assert that broken image URLs don't surface as uncaught JS exceptions during the test run. A request-wait can't substitute for that behavioral assertion, so the listener pattern is intentional here. | | 2 | Move helpers to a `vueNodes.getImageCompareHelper()` subclass | **Left as-is** | Helpers such as `setImageCompareValue` and `moveToPercentage` are only used in this file, making local encapsulation enough. Extracting them to a page object would increase the file/interface surface area and violate YAGNI; additionally, `AGENTS.md` clearly states to "minimize the exported values of each module. | | 3 | Use `TestIds` enum for test ID strings | **Fixed** – added `imageCompare` section to `TestIds` in `selectors.ts`; replaced all 8 inline string IDs in `imageCompare.spec.ts` with `TestIds.imageCompare.*` references | The project already has a `TestIds` convention for centralizing test IDs. Inline strings create drift risk between the Vue component and the test file. | | 4 | Move `expect.poll` bounding box check to helper/page object | **Left as-is** | This logic already lives inside `moveToPercentage`, which is a local helper. Moving it further to a page object is the same refactor as #2 above. | | 5 | Remove `// ---` style section header comments | **Fixed** – removed all 8 divider blocks from `imageCompare.spec.ts` | Consistent with project guidelines and your explicit preference. Test names already describe what each block does. | | 6 | Name magic numbers `400` and `350` | **Fixed** – introduced `minWidth = 400` and `minHeight = 350` constants in the test | Descriptive names make the constraint self-documenting and easier to update if the workflow asset changes. | <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: changes are limited to Playwright E2E test code and shared selector constants, with no production logic impacted. > > **Overview** > **E2E Image Compare tests now use centralized selectors.** Adds an `imageCompare` section to `TestIds` and updates `imageCompare.spec.ts` to reference `TestIds.imageCompare.*` instead of inline `data-testid` strings. > > Cleans up the spec by removing divider comments and naming the minimum size magic numbers (`minWidth`, `minHeight`) used in the node sizing assertion. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ece25be. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11260-test-address-review-nits-for-image-compare-E2E-3436d73d365081a69cacc1fff390035a) by [Unito](https://www.unito.io)
snomiao
pushed a commit
that referenced
this pull request
Apr 24, 2026
## Summary A follow-up PR of #11196. | # | Nit | Action | Reason | | :--- | :--- | :--- | :--- | | 1 | Replace `page.on('pageerror')` with request-wait | **Left as-is** | The `pageErrors` array is an accumulator checked at the end via `expect(pageErrors).toHaveLength(0)` – the goal is to assert that broken image URLs don't surface as uncaught JS exceptions during the test run. A request-wait can't substitute for that behavioral assertion, so the listener pattern is intentional here. | | 2 | Move helpers to a `vueNodes.getImageCompareHelper()` subclass | **Left as-is** | Helpers such as `setImageCompareValue` and `moveToPercentage` are only used in this file, making local encapsulation enough. Extracting them to a page object would increase the file/interface surface area and violate YAGNI; additionally, `AGENTS.md` clearly states to "minimize the exported values of each module. | | 3 | Use `TestIds` enum for test ID strings | **Fixed** – added `imageCompare` section to `TestIds` in `selectors.ts`; replaced all 8 inline string IDs in `imageCompare.spec.ts` with `TestIds.imageCompare.*` references | The project already has a `TestIds` convention for centralizing test IDs. Inline strings create drift risk between the Vue component and the test file. | | 4 | Move `expect.poll` bounding box check to helper/page object | **Left as-is** | This logic already lives inside `moveToPercentage`, which is a local helper. Moving it further to a page object is the same refactor as #2 above. | | 5 | Remove `// ---` style section header comments | **Fixed** – removed all 8 divider blocks from `imageCompare.spec.ts` | Consistent with project guidelines and your explicit preference. Test names already describe what each block does. | | 6 | Name magic numbers `400` and `350` | **Fixed** – introduced `minWidth = 400` and `minHeight = 350` constants in the test | Descriptive names make the constraint self-documenting and easier to update if the workflow asset changes. | <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: changes are limited to Playwright E2E test code and shared selector constants, with no production logic impacted. > > **Overview** > **E2E Image Compare tests now use centralized selectors.** Adds an `imageCompare` section to `TestIds` and updates `imageCompare.spec.ts` to reference `TestIds.imageCompare.*` instead of inline `data-testid` strings. > > Cleans up the spec by removing divider comments and naming the minimum size magic numbers (`minWidth`, `minHeight`) used in the node sizing assertion. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ece25be. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11260-test-address-review-nits-for-image-compare-E2E-3436d73d365081a69cacc1fff390035a) by [Unito](https://www.unito.io)
This was referenced May 4, 2026
4 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR configures vite to just copy everything from src directory to dist directory. Later we shall replace js with ts one by one.