-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
perf: T2I template editor preview #4574
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
Conversation
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.
Hey - 我发现了 1 个问题,并留下了一些高层次的反馈:
- 建议在对话框打开时只获取一次版本并进行缓存/复用,而不是在
dialog打开和每次refreshPreview时都调用syncPreviewVersion(),以避免重复的网络请求。 - 如果其他组件也需要应用版本信息,获取版本的逻辑(
syncPreviewVersion)可能更适合抽取到一个共享的 composable 或 service 中,这样之后就不用在多个地方重复处理/api/stat/version了。 - 建议不要在
syncPreviewVersion中直接使用console.warn,可以考虑通过现有的 UI 通知或日志上报机制来处理错误,这样失败信息就能更一致地展示给用户或监控系统。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider fetching the version once on dialog open and caching/reusing it rather than calling `syncPreviewVersion()` both on `dialog` open and every `refreshPreview`, to avoid redundant network requests.
- The version-fetching logic (`syncPreviewVersion`) might be better extracted into a shared composable or service if other components also need the app version, so you don't duplicate the `/api/stat/version` handling in multiple places later.
- Instead of using `console.warn` directly in `syncPreviewVersion`, consider routing errors through your existing UI notification or logging mechanism so failures are surfaced consistently to users or monitoring.
## Individual Comments
### Comment 1
<location> `dashboard/src/components/shared/T2ITemplateEditor.vue:299-302` </location>
<code_context>
}
+
+const previewData = computed(() => ({
+ text: tm('t2iTemplateEditor.previewText') || '这是一个示例文本,用于预览模板效果。\n\n这里可以包含多行文本,支持换行和各种格式。',
+ version: previewVersion.value
+}))
</code_context>
<issue_to_address>
**suggestion:** Using `||` means an intentionally empty translated string will fall back to the default text.
If `tm('t2iTemplateEditor.previewText')` returns an intentionally empty string, `||` will treat it as falsy and incorrectly fall back to the default Chinese text. To only fall back when the translation is missing (undefined/null), use nullish coalescing or an explicit check, e.g.:
```ts
const t = tm('t2iTemplateEditor.previewText')
text: t ?? defaultText
// or
text: t !== undefined ? t : defaultText
```
```suggestion
const previewData = computed(() => {
const t = tm('t2iTemplateEditor.previewText')
return {
text: t ?? '这是一个示例文本,用于预览模板效果。\n\n这里可以包含多行文本,支持换行和各种格式。',
version: previewVersion.value
}
})
```
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进以后的 Review。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Consider fetching the version once on dialog open and caching/reusing it rather than calling
syncPreviewVersion()both ondialogopen and everyrefreshPreview, to avoid redundant network requests. - The version-fetching logic (
syncPreviewVersion) might be better extracted into a shared composable or service if other components also need the app version, so you don't duplicate the/api/stat/versionhandling in multiple places later. - Instead of using
console.warndirectly insyncPreviewVersion, consider routing errors through your existing UI notification or logging mechanism so failures are surfaced consistently to users or monitoring.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider fetching the version once on dialog open and caching/reusing it rather than calling `syncPreviewVersion()` both on `dialog` open and every `refreshPreview`, to avoid redundant network requests.
- The version-fetching logic (`syncPreviewVersion`) might be better extracted into a shared composable or service if other components also need the app version, so you don't duplicate the `/api/stat/version` handling in multiple places later.
- Instead of using `console.warn` directly in `syncPreviewVersion`, consider routing errors through your existing UI notification or logging mechanism so failures are surfaced consistently to users or monitoring.
## Individual Comments
### Comment 1
<location> `dashboard/src/components/shared/T2ITemplateEditor.vue:299-302` </location>
<code_context>
}
+
+const previewData = computed(() => ({
+ text: tm('t2iTemplateEditor.previewText') || '这是一个示例文本,用于预览模板效果。\n\n这里可以包含多行文本,支持换行和各种格式。',
+ version: previewVersion.value
+}))
</code_context>
<issue_to_address>
**suggestion:** Using `||` means an intentionally empty translated string will fall back to the default text.
If `tm('t2iTemplateEditor.previewText')` returns an intentionally empty string, `||` will treat it as falsy and incorrectly fall back to the default Chinese text. To only fall back when the translation is missing (undefined/null), use nullish coalescing or an explicit check, e.g.:
```ts
const t = tm('t2iTemplateEditor.previewText')
text: t ?? defaultText
// or
text: t !== undefined ? t : defaultText
```
```suggestion
const previewData = computed(() => {
const t = tm('t2iTemplateEditor.previewText')
return {
text: t ?? '这是一个示例文本,用于预览模板效果。\n\n这里可以包含多行文本,支持换行和各种格式。',
version: previewVersion.value
}
})
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const previewData = computed(() => ({ | ||
| text: tm('t2iTemplateEditor.previewText') || '这是一个示例文本,用于预览模板效果。\n\n这里可以包含多行文本,支持换行和各种格式。', | ||
| version: previewVersion.value | ||
| })) |
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.
suggestion: 使用 || 会导致有意配置为空字符串的翻译也回退到默认文案。
如果 tm('t2iTemplateEditor.previewText') 返回的是一个有意设置的空字符串,|| 会把它当作 falsy,从而错误地回退到默认的中文文案。为了只在翻译缺失(undefined/null)时回退,建议使用空值合并运算符或显式判断,例如:
const t = tm('t2iTemplateEditor.previewText')
text: t ?? defaultText
// or
text: t !== undefined ? t : defaultText| const previewData = computed(() => ({ | |
| text: tm('t2iTemplateEditor.previewText') || '这是一个示例文本,用于预览模板效果。\n\n这里可以包含多行文本,支持换行和各种格式。', | |
| version: previewVersion.value | |
| })) | |
| const previewData = computed(() => { | |
| const t = tm('t2iTemplateEditor.previewText') | |
| return { | |
| text: t ?? '这是一个示例文本,用于预览模板效果。\n\n这里可以包含多行文本,支持换行和各种格式。', | |
| version: previewVersion.value | |
| } | |
| }) |
Original comment in English
suggestion: Using || means an intentionally empty translated string will fall back to the default text.
If tm('t2iTemplateEditor.previewText') returns an intentionally empty string, || will treat it as falsy and incorrectly fall back to the default Chinese text. To only fall back when the translation is missing (undefined/null), use nullish coalescing or an explicit check, e.g.:
const t = tm('t2iTemplateEditor.previewText')
text: t ?? defaultText
// or
text: t !== undefined ? t : defaultText| const previewData = computed(() => ({ | |
| text: tm('t2iTemplateEditor.previewText') || '这是一个示例文本,用于预览模板效果。\n\n这里可以包含多行文本,支持换行和各种格式。', | |
| version: previewVersion.value | |
| })) | |
| const previewData = computed(() => { | |
| const t = tm('t2iTemplateEditor.previewText') | |
| return { | |
| text: t ?? '这是一个示例文本,用于预览模板效果。\n\n这里可以包含多行文本,支持换行和各种格式。', | |
| version: previewVersion.value | |
| } | |
| }) |
This PR brings several improvements to the Template Editor preview:
/api/stat/version.Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
通过使用动态数据来替代硬编码的值,为 T2I 模板编辑器预览提供版本号和预览文本,从而改进预览效果。
新功能:
/api/stat/version接口获取模板预览版本,并在预览中动态显示该版本信息。改进:
Original summary in English
Summary by Sourcery
Improve the T2I template editor preview by using dynamic data for version and preview text instead of hardcoded values.
New Features:
Enhancements: