-
-
Notifications
You must be signed in to change notification settings - Fork 72
Template font manage #498
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: dev
Are you sure you want to change the base?
Template font manage #498
Conversation
Summary of ChangesHello @MakinoharaShoko, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求为模板编辑器引入了全面的字体管理功能。它允许模板创建者直接在模板的配置文件中定义和注册自定义字体,并将字体文件上传到资产目录。这些注册的字体随后可以在图形编辑器中被选择和使用,从而显著增强了模板的排版定制选项,为用户提供了更大的设计灵活性。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
这个 PR 实现了模板字体管理功能,允许用户在模板编辑器中配置自定义字体,整体实现得很好。代码结构清晰,功能完整,包括了前端 UI、API 定义和后端 DTO 的更新。
我发现了一些可以改进的地方:
- 一些新增的 UI 文本缺少了英语和日语的翻译。
- 在两个新组件中,使用
index或基于内容的字符串作为key来渲染列表,这可能会在列表项增删或重排时引发一些潜在的 UI bug。
具体的修改建议请见我的详细评论。
| msgid "font-family" | ||
| msgstr "" |
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.
检测到新增的 UI 文本缺少英文翻译。为了确保英文用户的体验,请为所有新增的 msgid 添加对应的 msgstr。
以下是所有缺失翻译的条目:
font-family上传字体文件(Upload font file)保存中...(Saving...)保存模板配置失败,请稍后重试(Failed to save template config, please try again later)字体文件 URL(Font file URL)字体文件上传失败,请重试(Font file upload failed, please try again)字体类型(Font type)字体配置(Font configuration)尚未配置任何字体(No fonts configured yet)新增字体(Add new font)模板名称不能为空(Template name cannot be empty)正在上传(Uploading)正在加载模板配置(Loading template config)请完整填写每个字体的信息(Please fill in the information for each font completely)配置模板(Configure template)
| msgid "font-family" | ||
| msgstr "" |
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.
| ) : ( | ||
| <div className={styles.fontList}> | ||
| {fonts.map((font, index) => ( | ||
| <div key={index} className={styles.fontItem}> |
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.
在 React 中使用数组索引 index 作为 key 可能会导致一些问题,尤其是在列表项可以被添加、删除或重新排序的情况下。当列表顺序改变时,React 可能会混淆组件实例,导致状态错乱或不正确的 UI 更新。
建议为每个字体项分配一个唯一的、稳定的 ID,并在渲染时使用该 ID 作为 key。这可以确保 React 能够准确地跟踪每个列表项。
例如,你可以在 FontFormState 类型中增加一个 id 字段,并在添加新字体时生成一个唯一 ID:
type FontFormState = TemplateFontConfigDto & { id: string };
const handleAddFont = () => {
setFonts((prev) => [
...prev,
{
id: crypto.randomUUID(), // Or another unique ID generation method
'font-family': '',
url: '',
type: defaultFontType,
},
]);
setErrorMessage(null);
};然后在 map 函数中使用 font.id 作为 key:
<div key={font.id} className={styles.fontItem}>
| <span className={styles.emptyHint}>{t`尚未添加任何字体`}</span> | ||
| )} | ||
| {effectiveFonts.map((font, index) => ( | ||
| <div key={`${index}-${font}`} className={styles.fontRow}> |
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.
使用 index 和 font 名称组合作为 key (key={${index}-${font}}) 同样存在一些问题。当字体名称被编辑时,key 会发生变化,这会导致 React 卸载并重新挂载该组件实例,从而丢失内部状态(例如输入框的焦点)。如果列表中存在重复的字体名称,key 也可能不是唯一的。
为了提高组件的健壮性,建议为每个字体项维护一个唯一的、稳定的 ID。
你可以将 fonts 状态的类型修改为 useState<{ id: string, name: string }[]>,并在解析或添加字体时为其分配唯一 ID。
例如:
// In useState initializer
const parsed = parseFontList(prop.propValue);
return parsed.length
? parsed.map(name => ({ id: crypto.randomUUID(), name }))
: DEFAULT_FONT_CHAIN.map(name => ({ id: crypto.randomUUID(), name }));
// In render
{effectiveFonts.map((font, index) => (
<div key={font.id} className={styles.fontRow}>
<Combobox
value={font.name}
// ... other props
/>
// ...
</div>
))}这样可以确保即使在编辑、重排或删除列表项时,React 也能正确地跟踪每个组件。
模板编辑器支持了编辑模板配置文件,并在配置文件中配置自定义字体。
字体可以上传到 template 的 assets 目录下。并自动填充字体 url。在注册字体后,相应的,引擎会读取字体并使其成为 Option 中的字体可选项。