feat: new option svgSideLoad = false

[keithharvey]

Allows the svgSideLoad option (default true) to force use of the svgPath in the href of the svg. The svgSideLoad option allows skipping dom insertion and makes it explicit how the svg is being handled since the default behavior is surprising when passing in an svgPath (maybe I was not the only one confused by this? #325).

Context

The angularjs application I was working on was rendering trumbowyg inside a dynamic tab. Switching tabs would cause the dom reference technique to fail whenever it re-rendered via client side rendering, tab switches. I saw the option for svgPath and immediately assumed I could just pass in an explicit url. Instead I found the current page URL always and the current implementation, which exists to allow styling of the svg (#253 (comment)). Editing the svg html and setting the href to the svgPath I passed into to trumbowyg revealed that just letting the browser do it solved my re-rendering problem.

In my case I'm stuck with a legacy app and just want them to work so the browser caching them is fine.

[Alex-D]

Hey!

I have read, and re-read your PR, and I'm sorry but I did not understand the need.

Why can you just not import SVG in HTML of your tab? Maybe before Trumbowyg init. Since I did not understand the point of your code, I cannot merge it. Also I cannot write documentation for it.

I have already an idea to avoid all those issues in v3, because SVG loading issues are annoying.

[keithharvey]

Why can you just not import SVG in HTML of your tab?

You totally can. If you inject it into your layout it does work.

I have already an idea to avoid all those issues in v3, because SVG loading issues are annoying.

It is needed because it resolves SVG loading issues (see below) and lets me just specify a spot on the server for my custom SVGs (or embed them in the page). I get your resistance to two ways to load svgs in the plugin. In the application I'm working on, I intentionally don't cache html and as a rule try to have my entry points thin. So in this app, I'm either going to have the plugin ajax fetch it or have the browser do it, both of which will be cached pretty quickly.

The reason I went ahead and opened the PR is how confusing the API is. svgPath: '/icons.svg' suggests that your svgs would use that as a data url, nothing complicated, browser handles caching, that seems cleanest. We can't break the existing use case in v2 (for looking up the svg in DOM by default) but we can make the behavior and API less opaque by making it clear that by default, the svgs are side loaded, and giving consumers the ability to opt out. Pretty minor but just being able to say "nevermind let the browser handle it" in the api with svgSideLoad: false, and then in your docs describing the svgSideLoad: boolean (default true) and how that works, without a breaking change, might help others learning in v2.

I have already an idea to avoid all those issues in v3, because SVG loading issues are annoying.

That's fantastic.

---

There are probably other ways to fix the ajax loader so this is unnecessary. There is currently a race condition whenever it calls init prior to the $.ajax svg call completing. Subsequent init calls (like navigating away and back again) on re-renders are totally fine.

https://github.com/Alex-D/Trumbowyg/blob/develop/src/trumbowyg.js#L449

I'm trying to work out ways to fix that but most of them involve wrapping up the init() calls into a promise and eliminating the race condition.

[keithharvey]

Had a few force pushes messed up there but updated with your feedback and rolled it into one commit. If you would you like me to just use options.svgSideLoad instead of the variable just let me know.

[Alex-D]

I'm thinking that it should be better to init the option in the default options object, to true, allowing us to remove that check ^^

[keithharvey]

Went ahead and did that. I did have to move t.o initialization above the svg initialization because the defaults weren't quite pulled in yet, but that all seems to work.

[Alex-D]

Looks good
I will take some time to test it in real context + write the doc ASAP :)

Thank you!

[keithharvey]

Thank you! I really appreciate your thoughtfulness.

Edit: never mind will separate this out into another ticket

[keithharvey]

@Alex-D My colleague added a fix we found. You can reproduce it by adding a new 3x3 table using the table plugin, then clicking off to the side of the top left cell. The wrapper fires but doesn't know about the table yet, so ends up wrapping each td in a p tag, which chrome redraws for you when it discovers how invalid it is.

[Alex-D]

1/ I think it should be in another PR
2/ I think it's due to missing syncCode in table plugin, so this is not a fix but a workaround ^^'

I need to take the time to look at the original option, I was working on something else + computer issues :/

[keithharvey]

2/ Ok, I forced pushed it from the develop branch and moved the hack.
1/ Ok, I created #1122. Sorry about that. I'm very curious but won't PR until I have something to contribute and right now it's a bit of a mystery to me, personally. I'm happy to pursue it further with more information.

[ydubey]

Hi,

I am struggling with what I think is because of the URL in svg > path element. I am using angular with core-ui. The icons do not show when editor component is inside a tab. Looking at the network traffil, there is a http get request to the current pages url which is returned with 404. I suspect, this is when the svg icons are being rendered.

I dont have much context of how svg is being loaded, but reading the description I think svgSideLoad is the solution.

I tried using latest 2.22.0, with svgSideLoad: false, but that doesn't seem to make any difference to the url on svg > path element.

`
$("#templateEditor").trumbowyg({
lang: 'en',
svgSideLoad: false,
svgPath: '/assets/icon/icons.svg',
removeformatPasted: true
});

`
Am I missing something?

[Alex-D]

Yes, sorry, I've renamed the option ^^
It's svgAbsoluteUseHref now :)

I did not update the documentation, for now, I will work on it soon.

[ydubey]

Hi,

Thanks for the reply. I tried both
svgAbsoluteUseHref: false
and

svgAbsoluteUseHref: '/assets/icon/icons.svg'
none seemed to make any difference to the href in svg > use. Is this change part of 2.22.0 version?

Thanks

[Alex-D]

Use svgAbsoluteUseHref: true

[ydubey]

Hi sorry so say, even that doesn't work. Let me try to create a sample angular project for you to debug and see whats going on.

Thanks for your help.

[Alex-D]

That's not an instance option. It's a global option, like svgPath.

$.trumbowyg.svgPath = '/assets/icon/icons.svg'
$.trumbowyg.svgAbsoluteUseHref = true

This will apply to all Trumbowyg instances.

[ydubey]

Thanks, that worked.