add：ma-icon-panel自定义svg包 #418
Conversation
dosubot
bot
added
the
size:M
WalkthroughThe changes in Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
web/src/components/ma-icon-picker/ma-icon-panel.vue (1)
45-49: Consider performance and error handling improvements for SVG loading.The eager loading of all SVG files could impact initial load performance. Consider these improvements:
- Use lazy loading (
eager: false) for better initial load performance- Add validation for SVG files
- Add TypeScript types for the imported files
-const customIcons = import.meta.glob('@/assets/icons/*.svg', { eager: true }) +const customIcons = import.meta.glob<{ default: string }>('@/assets/icons/*.svg', { + eager: false, + transform: (svg) => { + // Basic SVG validation + if (!svg.default?.includes('<svg')) { + throw new Error('Invalid SVG file'); + } + return svg; + } +})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
web/src/components/ma-icon-picker/ma-icon-panel.vue(2 hunks)
🔇 Additional comments (1)
web/src/components/ma-icon-picker/ma-icon-panel.vue (1)
45-49: Verify SVG content security.
Loading external SVG files could pose security risks if they contain malicious scripts. Consider implementing SVG sanitization.
| <template v-for="item in updatedData" :key="item.prefix"> | ||
| <el-tab-pane :label="item.info.name" :name="item.prefix" class="mt-2"> | ||
| <div class="relative"> | ||
| <OverlayScrollbarsComponent class="h-[400px] p-3 pl-2 pr-4"> | ||
| <div class="icon-content"> | ||
| <template v-for="icon in pageData"> | ||
| <div | ||
| class="icon-item" :class="{ | ||
| active: `${item.prefix}:${icon}` === model, | ||
| active: currentName === 'custom' ? icon === model : `${item.prefix}:${icon}` === model, | ||
| }" | ||
| @click="selected(`${item.prefix}:${icon}`)" | ||
| @click="selected(currentName === 'custom' ? icon : `${item.prefix}:${icon}`)" | ||
| > | ||
| <ma-svg-icon :name="`${item.prefix}:${icon}`" :size="26" /> | ||
| <ma-svg-icon :name="currentName === 'custom' ? icon : `${item.prefix}:${icon}`" :size="26" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify custom icon handling logic and add error handling.
The current implementation has repetitive conditions and lacks error handling. Consider:
- Extracting the icon name resolution logic
- Adding loading states for dynamic icons
- Handling icon load failures
+const getIconName = (icon: string) => currentName.value === 'custom' ? icon : `${item.prefix}:${icon}`
<template v-for="icon in pageData">
<div
class="icon-item"
- :class="{
- active: currentName === 'custom' ? icon === model : `${item.prefix}:${icon}` === model,
- }"
- @click="selected(currentName === 'custom' ? icon : `${item.prefix}:${icon}`)"
+ :class="{ active: getIconName(icon) === model }"
+ @click="selected(getIconName(icon))"
>
- <ma-svg-icon :name="currentName === 'custom' ? icon : `${item.prefix}:${icon}`" :size="26" />
+ <ma-svg-icon
+ :name="getIconName(icon)"
+ :size="26"
+ @error="handleIconError"
+ v-loading="isLoading(icon)"
+ />
</div>
</template>Committable suggestion skipped: line range outside the PR's diff.
| // 将自定义图标追加到 data 中 | ||
| function appendCustomIcons() { | ||
| const customData = { | ||
| prefix: 'custom', | ||
| info: { | ||
| name: '自定义图标', | ||
| total: customIconList.length, | ||
| version: '1.0.0', | ||
| }, | ||
| icons: customIconList, | ||
| } | ||
| return [...data, customData] | ||
| } | ||
|
|
||
| // 将处理后的数据赋值回 data | ||
| const updatedData = appendCustomIcons() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve custom icon data structure implementation.
Several improvements are needed:
- The name "自定义图标" should use i18n
- Add validation for icon name collisions
- Consider making the version configurable
function appendCustomIcons() {
+ // Check for duplicate icon names
+ const existingIcons = new Set(data.flatMap(d => d.icons))
+ const duplicates = customIconList.filter(icon => existingIcons.has(icon))
+ if (duplicates.length > 0) {
+ console.warn(`Duplicate icon names found: ${duplicates.join(', ')}`)
+ }
+
const customData = {
prefix: 'custom',
info: {
- name: '自定义图标',
+ name: t('customIcons'), // Add this to i18n
total: customIconList.length,
- version: '1.0.0',
+ version: import.meta.env.VITE_APP_VERSION || '1.0.0',
},
icons: customIconList,
}
return [...data, customData]
}Committable suggestion skipped: line range outside the PR's diff.
|
Awesome work, congrats on your first merged pull request! 祝贺你的第一个拉取请求被合并！ |
add：自动加载assets/icons目录下的svg图标
Summary by CodeRabbit
New Features
Bug Fixes