-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Conversation
@@ -11,6 +11,15 @@ | |||
"animate/fill": { | |||
"attribOptions": ["freeze", "remove"] | |||
}, | |||
"animateColor/fill": { | |||
"attribOptions": ["freeze", "remove"] | |||
}, |
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.
Why are there separate entries for each [tag]/fill
case when the options are the same (i.e. ["freeze", "remove"]
) ?
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.
Earlier it was using alias in such situations, that was removed for readability. I should use type
property for each of these attributes.
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.
I'm not sure what you mean. Locally, I replaced all of the */fill
entries with:
"fill": {
"attribOptions": ["freeze", "remove"]
},
and it seems to work correctly.
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.
I was saying that animate*/fill
should only hint freeze
and remove
, so we can use fill
for color names.
I'm not sure what you mean.
"animate*/fill": {
"attribOptions": ["freeze", "remove"]
},
"fill": {
"type": "color"
}
Then we can use its type
property for hinting color names.
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.
OK, that makes sense. Do you want to add color name support in this PR so that it's clear why it was done this way?
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.
Yes, color name support will be added in this PR.
@redmunds Added color names support. |
Added color names support. SVG code hint refactor Added transparent, currentColor SVG code hints refactoring
Looks good. Merging. |
fill
values toanimateTransform
,animateMotion
,animateColor
andset
tags.