Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

SVG code hints cleanup #10403

Merged
merged 1 commit into from
Jan 22, 2015
Merged

SVG code hints cleanup #10403

merged 1 commit into from
Jan 22, 2015

Conversation

sprintr
Copy link
Contributor

@sprintr sprintr commented Jan 17, 2015

  • Attributes are now unique.
  • Added fill values to animateTransform, animateMotion, animateColor and set tags.
  • Named Color Support

@@ -11,6 +11,15 @@
"animate/fill": {
"attribOptions": ["freeze", "remove"]
},
"animateColor/fill": {
"attribOptions": ["freeze", "remove"]
},
Copy link
Contributor

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"]) ?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor Author

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.

@sprintr
Copy link
Contributor Author

sprintr commented Jan 21, 2015

@redmunds Added color names support.

Added color names support.

SVG code hint refactor

Added transparent, currentColor

SVG code hints refactoring
@redmunds
Copy link
Contributor

Looks good. Merging.

redmunds added a commit that referenced this pull request Jan 22, 2015
@redmunds redmunds merged commit b071b24 into adobe:master Jan 22, 2015
@sprintr sprintr deleted the svg-refactor branch January 22, 2015 01:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants