fix(dashboard): include missing vuetify mdi icons#6970
fix(dashboard): include missing vuetify mdi icons#6970RC-CHN merged 1 commit intoAstrBotDevs:masterfrom
Conversation
Export the required icon set and expand it with icons used by Vuetify internals that are not detected by static source scans. Regenerate the MDI subset assets and update tests to assert that all required icons are always included and deduplicated.
dosubot
Bot
added
the
size:L
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 and resolves the issue of missing Material Design Icons (MDI) in the dashboard by explicitly defining and including a broader range of icons that Vuetify internally relies upon. This ensures a complete and consistent visual experience across the application, preventing UI glitches caused by absent icons. 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
|
dosubot
Bot
added
the
area:webui
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
REQUIRED_ICONSset is getting fairly large; consider grouping it into semantically named subsets (e.g. pagination, alerts, selection controls) or adding brief comments per block so future maintainers can see why each group of icons is required by Vuetify internals.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `REQUIRED_ICONS` set is getting fairly large; consider grouping it into semantically named subsets (e.g. pagination, alerts, selection controls) or adding brief comments per block so future maintainers can see why each group of icons is required by Vuetify internals.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request expands the set of required Material Design Icons (MDI) by adding numerous new icons to dashboard/scripts/subset-mdi-font.mjs and updating the corresponding CSS. The REQUIRED_ICONS variable is now exported, and the test suite dashboard/tests/subsetMdiFont.test.mjs has been refactored to use this exported set for more comprehensive and maintainable icon scanning and deduplication tests. Feedback suggests sorting the REQUIRED_ICONS list for better maintainability and simplifying several test assertions for conciseness and robustness.
| "mdi-radiobox-blank", | ||
| "mdi-radiobox-marked", | ||
| "mdi-menu-down", | ||
| "mdi-menu-right", | ||
| "mdi-check-circle", | ||
| "mdi-information", | ||
| "mdi-alert-circle", | ||
| "mdi-close-circle", | ||
| "mdi-chevron-down", | ||
| "mdi-chevron-up", | ||
| "mdi-chevron-left", | ||
| "mdi-chevron-right", | ||
| "mdi-check", | ||
| "mdi-close", | ||
| "mdi-checkbox-marked", | ||
| "mdi-checkbox-blank-outline", | ||
| "mdi-minus-box", | ||
| "mdi-circle", | ||
| "mdi-arrow-up", | ||
| "mdi-arrow-down", | ||
| "mdi-menu", | ||
| "mdi-pencil", | ||
| "mdi-star-outline", | ||
| "mdi-star", | ||
| "mdi-star-half-full", | ||
| "mdi-cached", | ||
| "mdi-page-first", | ||
| "mdi-page-last", | ||
| "mdi-unfold-more-horizontal", | ||
| "mdi-paperclip", | ||
| "mdi-plus", | ||
| "mdi-minus", | ||
| "mdi-calendar", | ||
| "mdi-eyedropper", | ||
| "mdi-cloud-upload", |
There was a problem hiding this comment.
For improved maintainability, it's good practice to keep long lists like this sorted. Please sort the icons in REQUIRED_ICONS alphabetically. This makes it much easier to check for duplicates or find a specific icon in the future.
"mdi-alert-circle",
"mdi-arrow-down",
"mdi-arrow-up",
"mdi-cached",
"mdi-calendar",
"mdi-check",
"mdi-check-circle",
"mdi-checkbox-blank-outline",
"mdi-checkbox-marked",
"mdi-chevron-down",
"mdi-chevron-left",
"mdi-chevron-right",
"mdi-chevron-up",
"mdi-circle",
"mdi-close",
"mdi-close-circle",
"mdi-cloud-upload",
"mdi-eyedropper",
"mdi-information",
"mdi-menu",
"mdi-menu-down",
"mdi-menu-right",
"mdi-minus",
"mdi-minus-box",
"mdi-page-first",
"mdi-page-last",
"mdi-paperclip",
"mdi-pencil",
"mdi-plus",
"mdi-radiobox-blank",
"mdi-radiobox-marked",
"mdi-star",
"mdi-star-half-full",
"mdi-star-outline",
"mdi-unfold-more-horizontal"| for (const requiredIcon of REQUIRED_ICONS) { | ||
| assert.ok(icons.has(requiredIcon)); | ||
| } |
There was a problem hiding this comment.
This loop is redundant. The assert.deepEqual(icons, expectedIcons) on line 91 already verifies that all required icons are present in the icons set. Removing this loop will make the test more concise without losing any coverage.
| for (const requiredIcon of REQUIRED_ICONS) { | ||
| assert.ok(icons.has(requiredIcon)); | ||
| } | ||
| assert.equal(icons.size, REQUIRED_ICONS.size); |
There was a problem hiding this comment.
These assertions can be simplified. Instead of iterating through all required icons and then checking the size, you can use a single assert.deepEqual to compare the resulting icons set with the REQUIRED_ICONS set directly. This is more concise and robust, as it verifies both the content and size in one step.
assert.deepEqual(icons, REQUIRED_ICONS);| const icons = [...scanUsedIcons(collectFiles(tmp, ['.vue']))]; | ||
| assert.equal(icons.filter(icon => icon === 'mdi-radiobox-marked').length, 1); | ||
| assert.ok(icons.includes('mdi-radiobox-blank')); | ||
| assert.equal(icons.filter(icon => icon === requiredIcon).length, 1); | ||
| for (const builtInRequiredIcon of REQUIRED_ICONS) { | ||
| assert.ok(icons.includes(builtInRequiredIcon)); | ||
| } | ||
| assert.ok(icons.includes('mdi-home')); |
There was a problem hiding this comment.
This test's logic can be greatly simplified. Since scanUsedIcons returns a Set, which handles deduplication automatically, you don't need to convert it to an array and check for duplicates manually. A single assert.deepEqual against an expected Set is a much cleaner and more direct way to verify that all expected icons are present and correctly deduplicated.
const icons = scanUsedIcons(collectFiles(tmp, ['.vue']));
const expectedIcons = new Set([...REQUIRED_ICONS, 'mdi-home']);
assert.deepEqual(icons, expectedIcons);
1 participant
Export the required icon set and expand it with icons used by Vuetify internals that are not detected by static source scans.
Regenerate the MDI subset assets and update tests to assert that all required icons are always included and deduplicated.
Modifications / 改动点
在dashboard/scripts/subset-mdi-font.mjs中导入vuetify隐式依赖的icon以解决图标缺失问题
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
Ensure the dashboard MDI icon subset always includes Vuetify’s implicit icon dependencies and remains deduplicated.
Bug Fixes:
Enhancements:
Tests: