-
Notifications
You must be signed in to change notification settings - Fork 403
[i18n] Fix missing subscription.* translations #6610
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
base: main
Are you sure you want to change the base?
Conversation
🎭 Playwright Test Results⏰ Completed at: 11/06/2025, 04:54:48 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/06/2025, 04:32:01 PM UTC 🔗 Links🎨 Chromatic Visual Tests🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.25 MB (baseline 3.25 MB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 792 kB (baseline 792 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 8.18 kB (baseline 8.18 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 293 kB (baseline 293 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
UI Components — 12.6 kB (baseline 12.6 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 10.4 kB (baseline 10.4 kB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 1.07 kB (baseline 1.07 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 5.32 MB (baseline 5.32 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 2.55 MB (baseline 2.55 MB) • ⚪ 0 BBundles that do not match a named category
|
## Summary As the commit says, the model loaders were broken in cloud if you enabled Vue Nodes (not a thing I think user does yet). This fixes it by configuring the `WidgetSelectDropdown` to load so the user load models like they would load a input or output asset. ## Review Focus Probably `useAssetWidgetData` to make sure it's idomatic. This part of [assetsStore](https://github.com/Comfy-Org/ComfyUI_frontend/pull/6607/files#diff-18a5914c9f12c16d9c9c3a9f6d0e203a9c00598414d3d1c8637da9ca77339d83R158-R234) as well. ## Screenshots <img width="1196" height="1005" alt="Screenshot 2025-11-05 at 5 34 22 PM" src="https://github.com/user-attachments/assets/804cd3c4-3370-4667-b606-bed52fcd6278" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6607-fix-use-WidgetSelectDropdown-for-models-2a36d73d36508143b185d06d736e4af9) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <[email protected]>
…6609) ## Summary - Implement graceful handling of Vite preload errors that occur when assets are deleted after new deployments - Auto-reload when safe (no unsaved changes), show confirmation dialog when user has unsaved work - Add i18n support for user-friendly error messages ## Implementation Details - Add `vite:preloadError` event listener in App.vue - Smart reload logic: check `app.vueAppReady` and `workflowStore.activeWorkflow?.isModified` - User confirmation dialog using existing `dialogService.confirm` - Comprehensive i18n keys for title and message ## Background This addresses the issue described in [Vite documentation](https://vite.dev/guide/build.html#load-error-handling) where users encounter import errors when hosting services delete old assets after new deployments. [screen-capture (1).webm](https://github.com/user-attachments/assets/beed3b8e-6f32-4288-a560-55da391a79a1) 🤖 Generated with [Claude Code](https://claude.ai/code) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6609-fix-Handle-vite-preloadError-for-graceful-deployment-asset-updates-2a36d73d365081a0b3adeac9fcd1e1dc) by [Unito](https://www.unito.io)
## Summary Use semantic tokens instead of colors ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6304-style-update-Vue-node-designs-to-use-semantic-tokens-2986d73d365081f69acce7793a218699) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude <[email protected]>
## Summary Re-adding some strings that got dropped in the merge. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6613-fix-re-add-translations-dropped-in-6564-2a36d73d3650818ca617cb5bddd11bc7) by [Unito](https://www.unito.io)
…-Org/ComfyUI_frontend into version-bump-fix-subscription-i18n
The previous workflow used git stash/pop which could fail silently due to the '|| true' clause, causing generated translations to be lost. This commit simplifies the git operations by removing the stash/checkout/pop sequence and instead directly checking out the branch and adding changes.
| git diff --staged --quiet || git commit -m "Update locales" | ||
| git push origin HEAD:${{ github.head_ref }} | ||
| if ! git diff --staged --quiet; then | ||
| git commit -m "Update locales" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[quality] high Priority
Issue: Potential error silencing with || true
Context: The removal of git stash pop || true removes error handling, but this is actually good as it was masking real failures
Suggestion: The new approach is better - explicit conditional commit logic is much clearer than git stash operations
| git checkout -B ${{ github.head_ref }} origin/${{ github.head_ref }} | ||
| # Apply the stashed changes if any | ||
| git stash pop || true | ||
| git checkout ${{ github.head_ref }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[architecture] low Priority
Issue: Git workflow simplification is good but comments should explain the change
Context: Removing git stash/pop operations simplifies the workflow but the rationale could be better documented
Suggestion: The commit message explains it well, but adding a comment in the YAML would help future maintainers understand why this approach was chosen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comprehensive PR Review
This review is generated by Claude. It may not always be accurate, as with human reviewers. If you believe that any of the comments are invalid or incorrect, please state why for each. For others, please implement the changes in one way or another.
Review Summary
PR: [i18n] Fix missing subscription.* translations (#6610)
Impact: Minimal changes across 3 files - primarily workflow fixes and locale updates
Issue Distribution
- Critical: 0
- High: 1
- Medium: 0
- Low: 1
Category Breakdown
- Architecture: 1 issue
- Security: 0 issues
- Performance: 0 issues
- Code Quality: 1 issue
Key Findings
Architecture & Design
This PR appropriately addresses a specific i18n workflow issue where subscription translations were missing. The approach of creating trigger files and simplifying git operations in the workflow is sound architectural practice for CI/CD pipelines.
Workflow Improvements
The git workflow simplification removing stash/pop operations is a good improvement that eliminates potential failure points. The previous || true operator was masking failures that could result in lost translations.
Security Considerations
No security issues identified in the workflow changes. The changes maintain proper authentication and authorization patterns.
Performance Impact
Minimal performance impact - the workflow changes should actually be more reliable and potentially faster by removing unnecessary git operations.
Positive Observations
- Clear problem identification and targeted solution
- Good commit message documentation explaining the rationale
- Minimal, focused changes that don't introduce unnecessary complexity
- Proper use of conditional logic in GitHub Actions workflow
References
Next Steps
- Consider adding workflow documentation comments as suggested in inline feedback
- Monitor the i18n workflow execution to ensure translations are generated correctly
- Verify the trigger mechanism works as expected for future i18n updates
This is a comprehensive automated review focused on the i18n workflow improvements. The changes are minimal and targeted, addressing a specific operational issue.
Summary
Fixes missing translations for
subscription.*keys that were added after the last automatic locale update.Problem
The
subscription.titleUnsubscribedkey was added on Nov 1, 2025 (commit f2aea9c), but the last automatic locale update ran on Oct 30, 2025. The 1.32.2 release on Nov 6 did not include these translations because:.github/workflows/i18n-update-core.yaml) only runs onversion-bump-*branchesAs a result, the
subscription.*keys exist in English (src/locales/en/main.json) but are missing from all other locales (zh, zh-TW, ru, ja, ko, fr, es, ar, tr).Solution
This PR creates a
version-bump-*branch to trigger the automatic i18n workflow, which will:pnpm collect-i18nto collect i18n strings from the UIpnpm localeto generate translations using OpenAI (lobe-i18n)The workflow will generate translations for the missing keys:
subscription.titleUnsubscribedRelated
Testing
Once the i18n workflow completes, verify:
subscription.*keys are translated in all locales (zh, zh-TW, ru, ja, ko, fr, es, ar, tr)Workflow Details
The i18n workflow is configured to:
version-bump-*branches.i18nrc.cjs(including Chinese character requirements)┆Issue is synchronized with this Notion page by Unito