fix(ui): include vuetify radiobox icons#6892
Conversation
Add the radiobox icons used indirectly by Vuetify internals to the required MDI subset so they are kept during font generation. Regenerate the subset CSS and font files to prevent missing radio button icons at runtime.
Summary of ChangesHello, 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! This pull request addresses a user interface bug where Vuetify radio button icons were not rendering correctly. The problem stemmed from an optimization introduced in a previous PR that aggressively subsetted the Material Design Icons (MDI) font, inadvertently excluding icons used by Vuetify's internal components. The fix involves updating the font subsetting script to explicitly include these essential radiobox icons and regenerating the necessary CSS and font files, thereby restoring the intended visual appearance of radio buttons. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses an issue with missing radio button icons by explicitly including mdi-radiobox-blank and mdi-radiobox-marked in the MDI font subset. The approach of adding a REQUIRED_ICONS set in subset-mdi-font.mjs and initializing the usedIcons with it is a clean and effective solution. However, this change alters the behavior of the scanUsedIcons function, which now causes existing unit tests to fail. It's important to update the tests to reflect this new behavior to maintain the integrity of the test suite. I've left a specific comment regarding the necessary test updates.
| export function scanUsedIcons(sourceFiles) { | ||
| const iconPattern = /mdi-[a-z][a-z0-9-]*/g; | ||
| const usedIcons = new Set(); | ||
| const usedIcons = new Set(REQUIRED_ICONS); |
There was a problem hiding this comment.
This change correctly initializes the usedIcons set with the required icons. However, it alters the behavior of scanUsedIcons, which will now always include the icons from REQUIRED_ICONS in its result. This breaks existing unit tests for this function in dashboard/tests/subsetMdiFont.test.mjs that assert the size of the returned set (e.g., expecting an empty set when no icons are found in files). Please update the relevant tests to account for this new behavior to ensure the test suite remains accurate and passes.
Add the radiobox icons used indirectly by Vuetify internals to the required MDI subset so they are kept during font generation.
Regenerate the subset CSS and font files to prevent missing radio button icons at runtime.
fix of #6882 & #6881
Modifications / 改动点
问题由 #6532 引入,该PR为了优化前端资源体积,将MDI图标字体从387KB削减至12.7KB,但字体子集化脚本遗漏了Vuetify Radio组件的默认图标:mdi-radiobox-marked 和 mdi-radiobox-blank。
在 dashboard/scripts/subset-mdi-font.mjs 中手动添加这两个图标到必需图标列表,然后重新构建,即可解决该问题
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Bug Fixes: