Skip to content
This repository has been archived by the owner on Oct 9, 2020. It is now read-only.

Fix: idPrefix regex to support style attribute #90

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

koox00
Copy link

@koox00 koox00 commented May 13, 2019

Currently createIdPrefix for tag attributes only works if the url(#foo) is the only value.

e.g:

<path mask="url(#foo")>

This commit adds support for the following.

<g style="mask: url(#foo)">

@jsf-clabot
Copy link

jsf-clabot commented May 13, 2019

CLA assistant check
All committers have signed the CLA.

@koox00 koox00 changed the title Fix: idPrefix regex to support style attr Fix: idPrefix regex to support style attribute May 14, 2019
@@ -131,7 +131,7 @@ function createClassPrefix(classPrefix) {
}

function createIdPrefix(idPrefix) {
var url_pattern = /^url\(#.+\)$/i;
var url_pattern = /url\(#.+?\)/i;
Copy link

Choose a reason for hiding this comment

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

why add the question mark here ?

Copy link
Author

@koox00 koox00 Aug 19, 2019

Choose a reason for hiding this comment

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

@mathroc to enable lazy mode.

In greedy mode, the following
<g style="mask: url(#foo); color: rgba(0, 0, 0, 0, .1)">
would match
url(#foo); color: rgba(0, 0, 0, 0, .1)
but we want
url(#foo)

Currently createIdPrefix for tag attributes only works
if the url is the first value.

e.g: <path mask="url(#foo")>

This commit adds support also for the following.

<g style="mask: url(#foo)">
@@ -131,7 +131,7 @@ function createClassPrefix(classPrefix) {
}

function createIdPrefix(idPrefix) {
var url_pattern = /^url\(#.+\)$/i;
var url_pattern = /url\(#.+?\)/gi;

Choose a reason for hiding this comment

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

it wont work for this

<path mask="url(#foo")>

I guess keeping both can be better

Copy link
Author

@koox00 koox00 Feb 10, 2020

Choose a reason for hiding this comment

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

@anikethsaha It passes the tests though.
why wouldn't it work?

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

Successfully merging this pull request may close these issues.

4 participants