Skip to content

fix(svg): compatible with the original icon behavior. #3891

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.

Sign up for GitHub

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

Merged
zzcr merged 5 commits into from
Dec 9, 2025
+1,671 −1,671
Merged

fix(svg): compatible with the original icon behavior. #3891

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
89da9e4
fix(svg): compatible with the original icon behavior. when no primary…
shenjunjian Dec 9, 2025
1ff9479
fix(input): revert disabled color
shenjunjian Dec 9, 2025
6dd3e4b
fix(svg): revert svg
shenjunjian Dec 9, 2025
05083d2
fix(icons): 批量更新css var
shenjunjian Dec 9, 2025
6bc08d6
fix(icons): fix copy scripts
shenjunjian Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/vue-common/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,13 @@ export function svg({ name = 'Icon', component, filledComponent = null, deprecat
context,
extend
})
},
mounted() {
// 如果属性没有主副色，则遍历svg的所有内部元素，移除 fill 属性。
// 注意：移除后不能还原，所以： 如果用户使用时不传入主副色，后来修改主副色有值，也无法再把fill还原回来的。
if (this.firstColor === '' && this.secondColor === '') {
this.$el?.querySelectorAll('*').forEach((path) => path.removeAttribute('fill'))
}
}
Copy link

@coderabbitai coderabbitai bot Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Non-reactive behavior may break user expectations.

The mounted hook runs only once. If firstColor or secondColor props change from empty to non-empty values after mount, the fill attributes won't be restored. This breaks Vue's reactivity model and could confuse users who expect props to work reactively.

Consider using a watcher or computed property to handle this reactively:

-        },
-        mounted() {
-          // 如果属性没有主副色，则遍历svg的所有内部元素,移除 fill 属性。
-          // 注意：移除后不能还原,所以：  如果用户使用时不传入主副色,后来修改主副色有值,也无法再把fill还原回来的。
-          if (this.firstColor === '' && this.secondColor === '') {
-            this.$el?.querySelectorAll('*').forEach((path) => path.removeAttribute('fill'))
-          }
+        },
+        watch: {
+          firstColor: {
+            handler(newVal, oldVal) {
+              if (newVal === '' && this.secondColor === '') {
+                this.$el?.querySelectorAll('path, circle, rect, polygon, ellipse, line, polyline').forEach((el) => el.removeAttribute('fill'))
+              }
+            },
+            immediate: false
+          },
+          secondColor: {
+            handler(newVal, oldVal) {
+              if (newVal === '' && this.firstColor === '') {
+                this.$el?.querySelectorAll('path, circle, rect, polygon, ellipse, line, polyline').forEach((el) => el.removeAttribute('fill'))
+              }
+            },
+            immediate: false
+          }
+        },
+        mounted() {
+          // Remove fill attributes on initial mount if no colors specified
+          if (this.firstColor === '' && this.secondColor === '') {
+            this.$el?.querySelectorAll('path, circle, rect, polygon, ellipse, line, polyline').forEach((el) => el.removeAttribute('fill'))
+          }
         }

Alternatively, if the one-way behavior is intentional to match legacy icon behavior, document this clearly in the component's public API.

Committable suggestion skipped: line range outside the PR's diff.

})
)
Expand Down
Loading