Fill problem from Illustrator

Question

Fill problem from Illustrator

la9vka opened this issue 3 years ago · 31 comments

Is there a reason for this to be in the code? I have not experienced any adverse effects when deleting this, but it allows me to directly use illustrator made graphics without the fill problems.

svg_graphics.js line 187

.nr-dashboard-theme .nr-dashboard-template div.ui-svg path { fill: inherit; }

Answer 1 · 2021-09-16T22:44:30.000Z

Hi @la9vka,
We had a lot of problems in the past with fill colors. Not sure anymore about this one.
@Steve-Mcl : does this ring a bell to you?

Answer 2 · 2021-09-17T07:54:49.000Z

@bartbutenaers yes, it rings a distant bell. Something to do with dashboard styles and fill colour of shapes. Does the commit history for that line help?

Answer 3 · 2021-09-17T08:00:00.000Z

If this "opinionated" option is an issue, we could provide an advanced checkbox option of "inherit dashboard style" (set true by default)?

Answer 4 · 2021-09-18T21:35:31.000Z

Hey @Steve-Mcl ,
Thanks for joining!!!
That code has been introduced via this commit, which you have explained on Discourse at the time being.

Yes indeed I can add such a textbox. But - if your memory is better than mine - do you remember:

Whether it is enough to only generate or skip the line .nr-dashboard-theme .nr-dashboard-template div.ui-svg path { fill: inherit; } ?
An easy explanation that I can add to the info panel and readme page, to explain a bit simple what this option does?

Answer 5 · 2021-09-22T10:04:13.000Z

I had no problem with any other dashboard nodes removing it so a config option would be great.

"Disable SVG path fill inheritance" "Enabling this disabled the path fill inheritance. This may solve problems importing graphics from other programs."

Answer 6 · 2021-09-22T14:14:01.000Z

@bartbutenaers

I hate to say this, but to be 100% flexible, perhaps we add a style tab with it defaulted to ...

    div.ui-svg svg{
        color: var(--nr-dashboard-widgetColor);
        fill: currentColor !important;
    }
    div.ui-svg path {
        fill: inherit !important;
    }

That way, @la9vka can remove or tweak as required?

Alternatively, we simply enable/disable this "opinionated" style via a checkbox?

Answer 7 · 2021-09-22T16:08:59.000Z

@Steve-Mcl
Suppose I add a style tabsheet. If you have N svg nodes in a flow, how do we make sure that the the custom styles of each node are only applied to the svg shapes of that node? Because otherwise it might become confusing imho.

Answer 8 · 2021-09-22T16:14:15.000Z

Good question. I guess it would have to be scoped.

<style scoped>

And, thinking about it, even a checkbox to disable the styling would need the style to be scoped otherwise a second SVG node would still introduce the style.

Answer 9 · 2021-09-22T16:36:07.000Z

Scoped styles. Never heard about it. Will need to have a look first...
Thanks for the pointers!!!

Answer 10 · 2021-09-22T17:33:33.000Z

Have not tested it yet, but from this stackoverflow discussion I understand that scoped svg styles have been removed from the specs? But perhaps I am misinterpreting it. Did only a quick search...

Answer 11 · 2021-09-22T19:17:45.000Z

Indeed the scoped css doesn't seem to work in an SVG, as you can see in my test. Instead of a red and a blue circle, I get two blue circles ...

Answer 12 · 2021-09-22T19:30:51.000Z

Following this Stackoverflow discussion, it should be possible via a shadow DOM. But seems at first sight a rather big change of the svg-node to me...

Answer 13 · 2021-09-22T19:44:56.000Z

2 easier options to explore 1st...

If we don't add any other styles (other than in #45758bf) to the class .ui-svg we could (via a checkbox option) simply remove the class='ui-svg' meaning the specific SVG node won't inherit the styling.

Alternatively, we could scope the CSS by specificity. E.g use '#svggraphics_' + config.id + ' .ui-svg.etc.etc' in the CSS specifiers.

Answer 14 · 2021-09-22T20:23:15.000Z

Hmm that scoping by specificity is a nice workaround!
I assume we cannot add the '#svggraphics_' + config.id + automatically.
So the user will need to specify it manually all over the place.
Which will make the css less readable and understandable...

Answer 15 · 2021-09-22T20:53:17.000Z

I assume we cannot add the '#svggraphics_' + config.id + automatically.

Perhaps the CSS can be modified at server side already via the postcss-prefix-selector...

Answer 16 · 2021-09-22T21:58:36.000Z

I assume we cannot add the '#svggraphics_' + config.id + automatically.

I'm sure we could - by including '#svggraphics_' + config.id here and here as part of the specifier to make it more specific

e.g...

        var html = String.raw`
<style>
    .nr-dashboard-theme .nr-dashboard-template ` +  ' #svggraphics_' + config.id +  `  svg {
        color: var(--nr-dashboard-widgetColor);
        fill: currentColor;
    }
    .nr-dashboard-theme .nr-dashboard-template ` +  ' #svggraphics_' + config.id +  `  path {
        fill: inherit;
    }
</style>`
    + panzoomScripts + `
<div id='tooltip_` + config.id + `' display='none' style='z-index: 9999; position: absolute; display: none; background: cornsilk; border: 1px solid black; border-radius: 5px; padding: 2px;'></div>
<div class='ui-svg' id='svggraphics_` + config.id + `' ng-init='init(` + configAsJson + `)' style="width:100%; height:100%;">` + svgString + `</div>

`;              
        return html;

Answer 17 · 2021-09-23T22:07:56.000Z

Hi Steve,
Yes that is absolutely correct. We can do it indeed like that if we would implement a checkbox.
But assume a new tabsheet is added, where CSS style can be entered. I was wondering if I could youse that postcss-prefix-selector to add a prefix '#svggraphics_' + config.id` to every selector that has been entered in that new tabsheet. To make sure that the entered styles are only applied to the svg shapes in this svg-node only...

Answer 18 · 2021-09-24T09:55:47.000Z

Hmmmm.

An "easy" way of achieving that would be to wrap the CSS in an outer selector and run it through an scss processor.

Example...

https://www.cssportal.com/scss-to-css/

Let me think some more on this one.

Answer 19 · 2021-09-25T19:47:31.000Z

@Steve-Mcl : thanks for your help!
Don't hurry, because I have no time at the moment to implement this...

Answer 20 · 2021-10-13T16:14:15.000Z

run it through an scss processor.

@Steve-Mcl : would like to create a minor release, but would like to have this solved. Do you mean that I have to pass the default/entered CSS through a scss processor library on the server side, and then append it to the html string to send it to the client? I am not familiar with scss to be honest ...

Answer 21 · 2021-10-13T16:28:14.000Z

Yes. That was the suggestion. It gaves a way to maintain the original CSS while permitting the user to adjust it BUT keeping it scoped per instance.

I have no idea what this will add to the install size nor can I recommend a preprocessor (sorry)

Answer 22 · 2021-10-14T22:05:17.000Z

Hi @la9vka, @Steve-Mcl,

I have created a customizable-css branch, which adds the following:

An extra "CSS" tabsheet is added to the config screen.
The tabsheet has buttons to expand to fullscreen mode, format/beautify the CSS, reset the CSS to the default.
The original CSS is entered automatically by default.
The readme page has been extended with information and an example flow.
Migration code has been added to make sure existing nodes get the default original CSS.

I have spend most time to implement the fullscreen expand button for the CSS editor...

Due to limited free time, I have not been experimenting with all options. But I have used the postcss-prefix-selector library to add a prefix ("#svggraphics_" + config.id) to all the entered CSS selectors, in order to simulate scoped CSS (by applying the selectors only to the parent DIV element).

Since CSS is not really my cup of tea, I would appreaciate a lot if you guys could install that version and test it!! You can install it like this (within your .node-red folder):

npm install bartbutenaers/node-red-contrib-ui-svg#customizable-css

Some things I doubt about:

Not sure whether the CSS mode of the editor has been setup entirely correctly, because I get some warning on the default CSS:
Not sure whether the beautify rules are sufficient (because there are more options)?
Not sure whether my selector prefix is good enough, or should I use another one?
Others?

Thanks!

Answer 23 · 2021-10-15T22:01:12.000Z

Not sure whether the CSS mode of the editor has been setup entirely correctly, because I get some warning on the default CSS

ACE doesnt have great support for newer features like var(--x). Looks fine in monaco...

Not sure whether my selector prefix is good enough, or should I use another one?

Your CSS selector is not working - it is rendering as ...

#svggraphics_4a3ef0db9601ba54 div.ui-svg svg{
    color: var(--nr-dashboard-widgetColor);
    fill: currentColor !important;
}
#svggraphics_4a3ef0db9601ba54 div.ui-svg path {
    fill: inherit !important;
}

but they wont work. Need to be div#svggraphics_4a3ef0db9601ba54.ui-svg svg and div#svggraphics_4a3ef0db9601ba54.ui-svg path

Example...

NOTE

It is quire difficult to achieve that CSS so it might prove more efficient to wrap the whole output in a div with the class set to the ID of the current div wrapper e.g...

Then, what you have will work with a slight tweak...

const scopedCssString = postcss().use(prefixer({
          //prefix: "#svggraphics_" + config.id
          prefix: ".svggraphics_" + config.id // << use new wrapper div class name
        })).process(cssString).css;

Answer 24 · 2021-10-15T22:16:26.000Z

Also Bart, you are using ACE specific stuff so errors occur when monaco is used...

You might be better to store the selected TAB in node.tabs = RED.tabs.create({ e.g.

                    node.currentTab = tab.id; //ADD THIS

Answer 25 · 2021-10-16T20:54:11.000Z

Hey @Steve-Mcl,

A tremendous thank-you for this in depth code review, and the enhancement tips!!!!
I have update the branch and hopefully everything is fine now.

I also implemented another fix: the older nodes have a dot in their node id. Seems that the dot results in the style not being applied to the element. So I have replaced the dot by an underscore, which seems to solve it.

Will announce a beta on Discourse, so people can test it. Just to avoid that I have broken something in existing flows...

Answer 26 · 2021-10-16T21:00:57.000Z

I have merged it into the master branch.

Answer 27 · 2021-10-22T14:23:18.000Z

@la9vka,
It would be nice if you could install the 2.3.0 version from Github, using the following command (in your .node-red folder):

npm install bartbutenaers/node-red-contrib-ui-svg

I would like to know whether your problem is solved in this version?
And this version contains a large number of changes, so it would be nice if anybody could test it ...
Bart

Answer 28 · 2021-10-26T14:20:41.000Z

Not yet tested the larger svg's but looks good. One with modified css and one with original in the same dashboard group. Both with svg straight from illustrator.

Answer 29 · 2021-11-10T20:16:15.000Z

I have yet to find any issues on any of my old flows (previously run on unchanged 2.2.4). Please let me know if there is anything spesific you want me to test. Otherwise close the issue. Thanks for the amazing work :)

Answer 30 · 2021-11-15T22:28:07.000Z

Hi @la9vka,
Thanks for the offer!!!
To be honest I have no idea what could be wrong with those changes, so we will have to release it and wait for feedback...
But I am still waiting for feedback about issues reported (about another new feature in this 2.3.0 release), before I can publish it on NPM ;-(
Bart

Answer 31 · 2021-11-21T07:24:10.000Z

Hi @la9vka,

FYI: Version 2.3.0 is now available on NPM and in the palette