Skip to content
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.

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

Double fill or stroke property fix #40

Closed
wants to merge 1 commit into from
Closed

Double fill or stroke property fix #40

wants to merge 1 commit into from

Conversation

tabun-matadorov
Copy link

Hi.
I've noticed that vue-svgicon don't work in IE11 even with polyfill. I've found out that when you using original prop, it gives double fill or stroke property in XML: one from addOriginalColor and one from addColor, and xml parser in polyfill will throw an exception about invalid XML data.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 43.75% when pulling 3a927d6 on tabun-matadorov:master into 4015e16 on MMF-FE:master.

@Allenice
Copy link
Contributor

Your code will invalidate the "Overriding Original Color" feature. Please view the demo.

@Allenice
Copy link
Contributor

Fixed in v2.1.1

@tabun-matadorov
Copy link
Author

Thanks a lot, Allenice.
Just wanted to push another pull request with such code:

      path() {
        let _this = this
        
        function addOriginalColor (data) {
          let styleReg = /_fill=\"|_stroke="/gi
          return data.replace(styleReg, (styleName) => {
            return styleName && styleName.slice(1)
          })
        }

        function addColor (data) {
          let reg = /<(path|rect|circle|polygon|line|polyline|ellipse)\s[^/>]*/gi
          let i = 0
          return data.replace(reg, (match) => {
            let color = _this.colors[i++] || _this.colors[_this.colors.length - 1]
            // Use original colors
            if (_this.original && (!color || color === '_')) {
              return addOriginalColor(match);
            }
            
            let fill = _this.fill

            // if color is '_', ignore it
            if (color && color === '_') {
              return match
            }

            // if color start with 'r-', reverse the fill value
            if (color && color.indexOf('r-') === 0) {
              fill = !fill
              color = color.split('r-')[1]
            }

            let style = fill ? 'fill' : 'stroke'
            let reverseStyle = fill ? 'stroke' : 'fill'
            return match + `${style}="${color}" ${reverseStyle}="none" `
          })
        }

        let pathData = ''
        if (this.iconData) {
          pathData = this.iconData.data
          pathData = addColor(pathData);
        } else {
          // if no iconData, push to notLoadedIcons
          notLoadedIcons.push({
            name: this.iconName,
            component: this
          })
        }

        return pathData
      },

But it's already solved, so ... thanks again for a fast fixing for this issue.

@Allenice
Copy link
Contributor

Allenice commented Dec 16, 2017

I think your solution is better than mine. In addition, I suggest that your regular expression is best changed to this:

/<(path|rect|circle|polygon|line|polyline|ellipse)\s[^\/>]*/gi

"/" should be escaped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants