Implement currentColor support #158

Mrfence97 · 2020-10-22T21:47:09Z

This PR implements "currentcolour" support for the fill and stroke properties.

As per the SVG 1.1 (and 2.0) color spec, a user may specify a color property that is inherited by all child nodes. If the fill or stroke of any such nodes are set to "currentcolor", then the inherited color is used.

This is implemented by adding a color member to AtrributeState (that defaults to black). parseColor is amended to take in the current Context so that if "currentcolor" is passed, the color in the Context's AttributeState is returned.

Note the implementation is imperfect for two reasons:

The spec states that "currentcolor" may also be used by stop-color in gradients. Current browser implementations only ever use the color in-scope of the gradient as defined inside the <defs> tag; however, no Context is consumed when a Gradient node is created (as far as I can tell). This means there is no way to access this 'within <defs>' context. Therefore "currentcolor" is always black if used inside a gradient. Done: see commit/comment below.
Current browser implementations allow the current color to flow into the shadow-DOM if a <use> tag is used. However, the AttributeState is not passed down when a Use node is rendered, so the current color is inaccessible. So, again "currentcolor" is always black if used inside an element that is referenced with <use>. Done: see commit/comment below.

Hopefully someone can assist with these problems!

A number of test cases are also added.

- Add color ColorFill object to AttributeState - Updated when 'color' attribute is found - Update parseColor to include Context as an argument - If colourString is 'currentcolor' then uses color in AttributeState

- Running l2r, t2b, the rectangles in the spec svg test: - Fill is black if the color has not yet been set - Fill is black if the color is invalid - Fill is currentColor if color is set inside same node - Stroke is currentColor if color is set inside same node - Fill as currentColor works inside a nested node - Stroke as currentColor works inside a nested node - Fill as currentColor works inside a doubly nested node - Stroke as currentColor works inside a doubly nested node - currentColor uses the last defined color, including if that latest color is defined in the current node - Verify setting color has no impact on sibling node's currentColor

yGuy · 2020-10-23T07:06:09Z

Very nice! Thank you so much for that - I believe we should definitely consider the "use" use-case, because in my experiencethis is one of the most valuable usages of "currentColor". We might analyse the "defs" and search for occurences of "currentColor" and then opt-out of the PDF-sharing of the elements - I don't believe you can parameterized PDF elements - any ideas, @HackbrettXXX ?

- Allow currentcolor to be used in objects referenced in <use> - The current 'color' is passed down into the referenced object - ReferencesHandler getRendered API is changed to include the color in the key used to map an element to the rendered element - This is so a different PDF FormObject is used if a different color is present - Allow currentcolor to be used for 'stop-color's in <xGradient>'s - Support isn't perfect - In browser implementations of <stop>, the currentcolor always uses the color in the context of where the <stop> is defined; however the parent elements of the <stop> are never walked so we're unable to access this context! - POSSIBLE FIX: use getComputedStyle() of the <xGradient> element in the Gradient constructor to access the current color. However, this requires the SVG to actually be rendered by a browser.

Mrfence97 · 2020-10-25T21:38:14Z

I've pushed another commit that adds <use> and mostly adds 'stop-color' support.

`<use>`

The current color context is passed down to the refContext used to render the element referenced by the <use>. Note the API of getRendered() in ReferencesHandler has been changed to include the color. This color is used in the key to map an element to the rendered element as a different PDF FormObject is required if a different color is used! An optimisation (as @yGuy suggests) would be look in the 'defs' to see if currentcolor is actually used. If it's not, there's no need to create a different FormObject even if the active color is different! Currently this is not implemented.

`'stop-color'`

The stop-color support isn't perfect. If currentcolor is used within a <stop>, browser implementations always use the color active where the <stop> is declared (and having a different color when the gradient is actually used has no effect). However, as things stand there is no easy way to access the color value active where the <stop> is declared. Opening test/specs/current-color/spec.svg is a browser, you'll see the squares located at (3,0) and (3,2) have a magenta gradient (inherited from the color in the parents <defs>). Currently the package renders these squares as fully black.

A potential fix would be to use getComputedStyles() on the <stop> (or <xGradient>) element to access the current color, but this requires the SVG to be rendered by the browser! An alternative could be to walk up the parent elements of the <stop> (or <xGradient>) to work out which color is active.

HackbrettXXX · 2020-10-27T11:21:48Z

Thanks for these improvements.

`<use>`

I think it's fine how you implemented it for now. If you want you can implement the performance improvement, but this has low priority for me.

`stop-color`

I think it would be good if we would implement this like in the browser. Walking up the DOM from the gradient element should be ok. Make sure to use the getAttribute function from the 'utils/node.tsfile to check for thecolorproperty. You can use theStyleSheets` instance from the context where the gradient is referenced. This instance is valid everywhere.

- If "currentColor" is used in the 'stop-color' attribute of a <stop> node then the color is always taken from the context of the <stop> (and not wherever the gradient is used). - Adds a parent attribute to the SvgNode base class to track the parent SvgNode of every node. - If 'stop-color' is set to "currentColor", walks up the DOM (using the parent reference) to calculate the color context as at the <stop> node.

- If "color" is set on the <stop> element itself, ensure it is noted. - Also amends current-color test to verify this behaviour is correct.

Mrfence97 · 2020-10-29T20:37:29Z

`stop-color`

I've pushed another commit that adds full currentcolor support for stop-color. As was suggested, in the Gradient node's apply() method, the DOM is walked up to calculate the color set in the <stop>'s context.

To easily walk up the DOM, I've added a parent member to the base SvgNode class that is set for every node by parse() to its parent SvgNode.

HackbrettXXX

Thanks for implementing the support for stop-colors as well :)

src/nodes/gradient.ts

HackbrettXXX

Thanks for this awesome pull request! Will merge it now.

Mrfence97 added 3 commits October 22, 2020 21:32

Add currentColor support to fill/stroke attributes

32a92cd

- Add color ColorFill object to AttributeState - Updated when 'color' attribute is found - Update parseColor to include Context as an argument - If colourString is 'currentcolor' then uses color in AttributeState

Make currentcolor case-insensitive and add new test to tests.js

98065a2

Mrfence97 mentioned this pull request Oct 22, 2020

svg element fill="currentColor" #132

Closed

yGuy requested a review from HackbrettXXX October 23, 2020 07:06

Mrfence97 changed the title ~~Current color2~~ Implement currentColor support Oct 23, 2020

Mrfence97 added 2 commits October 29, 2020 20:12

Fix 'stop-color' corner case when "color" is set on the <stop> itself

e6d543f

- If "color" is set on the <stop> element itself, ensure it is noted. - Also amends current-color test to verify this behaviour is correct.

HackbrettXXX added the hacktoberfest-accepted label Oct 30, 2020

HackbrettXXX requested changes Nov 5, 2020

View reviewed changes

src/nodes/gradient.ts Outdated Show resolved Hide resolved

Only traverse ancestors once to calc contextColor

a0426f5

HackbrettXXX approved these changes Nov 6, 2020

View reviewed changes

Merge branch 'master' into current-color2

bbfc8f1

HackbrettXXX merged commit 6c2efd2 into yWorks:master Nov 6, 2020

Mrfence97 deleted the current-color2 branch November 11, 2020 10:24

micschro linked an issue Oct 24, 2022 that may be closed by this pull request

svg element fill="currentColor" #132

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement currentColor support #158

Implement currentColor support #158

Mrfence97 commented Oct 22, 2020 •

edited

yGuy commented Oct 23, 2020

Mrfence97 commented Oct 25, 2020 •

edited

HackbrettXXX commented Oct 27, 2020

Mrfence97 commented Oct 29, 2020

HackbrettXXX left a comment

HackbrettXXX left a comment

Implement currentColor support #158

Implement currentColor support #158

Conversation

Mrfence97 commented Oct 22, 2020 • edited

yGuy commented Oct 23, 2020

Mrfence97 commented Oct 25, 2020 • edited

<use>

'stop-color'

HackbrettXXX commented Oct 27, 2020

<use>

stop-color

Mrfence97 commented Oct 29, 2020

stop-color

HackbrettXXX left a comment

Choose a reason for hiding this comment

HackbrettXXX left a comment

Choose a reason for hiding this comment

Mrfence97 commented Oct 22, 2020 •

edited

Mrfence97 commented Oct 25, 2020 •

edited

`<use>`

`'stop-color'`

`<use>`

`stop-color`

`stop-color`