Refactor SVG filters to be singletons

[BeksOmega]

We have certain filters, such as the disabled filter, that are created multiple times with unique IDs (source code link). This was necessary because of a browser bug that has since been fixed. We should refactor these so that they're singletons, so we can reference them from CSS.

Fixing this will also involve actually referencing them from CSS.

This should be merged against the rc/v12.0.0 branch.

See the design doc for more info.

[jiya-singhal]

hey @BeksOmega
Can you assign this issue to me?

[johnnesky]

The screenshot feature of the dev-tools plugin will need to be adapted to be aware of the new location of the defs element: https://github.com/google/blockly-samples/blob/73d0f9d2ccbb471e6d233a608890ccbff732f779/plugins/dev-tools/src/screenshot.js#L104

[johnnesky]

TL;DR: We could make singleton SVG filters but if the motivation is to refer to them from CSS then I think it would be easier to keep the defs the same and just use CSS variables to refer to them.

There isn't an obvious place to put a shared global defs element. It has to be in an svg element to be effective, and svg elements are not supposed to go inside the head element, unlike css style elements. (Browsers would probably let us do it via JavaScript but if browsers directly load HTML that contains an svg in the head they would forcibly move it to the body.)

We could just put it at the beginning or end of the body element's children, and apply styles to visually hide the svg element such that it does not affect layout. We have to be careful, though, because using display: none; to hide an svg element also excludes its defs from being referenced elsewhere. (Which is another reason why we can't put the defs inside the head element which is usually hidden in this manner.) Instead, I propose we use the following style to hide the svg element:

<svg style="position: absolute !important; visibility: hidden !important; width: 0 !important; height: 0 !important; top: 0 !important; left: 0 !important;">
  <defs>...</defs>
</svg>

This would work, but we would have to hope that sneaking an svg element into the body won't mess up other expectations of the web app. For example, if the app has any CSS selectors like body :first-child then the hidden svg would steal that selector from another child. Similarly, if the app has any JavaScript that iterates over the children of the body, then our hidden svg element could confuse it.

The only location we can safely put defs is inside Blockly's own workspaces... which is where it is right now anyway. We could still make it a singleton by designating a single workspace to store the defs, but we'd have to be careful to transfer it to another workspace if we ever disposed of the first one. And as I mentioned previously, we'd still have to update the screenshot plugin to be aware of the current location of the singleton defs. Sounds complicated.

If, instead, we continue to allow each workspace to host its own copy of the defs, then we should also keep the random numbers inside the ids to avoid duplicate ids, in which case we would be back to where we started. Unless we put each workspace inside a shadow DOM, which would provide a scope for the ids, but that would be a pretty massive change.

I'm wondering what the advantages of singleton defs would be. The issue description says, "so we can reference them from CSS." I guess it's possible that we could put SVG fill attribute values inside CSS, like fill: url(#blocklyDisabledPattern);. Is this a sufficient motivation? An alternative way to accomplish this would be to define a CSS variable at the root of the workspace, like --blocklyDisabledPattern: url(#blocklyDisabledPattern123456789) and then refer to this variable in the global CSS, like fill: var(--blocklyDisabledPattern);. (More specifically, we should define the CSS variables on the block canvas element, which is the element that is cloned by the screenshot plugin, to ensure the variables are preserved. The screenshot plugin already copies the global styles and the workspace local defs.)

So in summary, I think our options are:

1. Put the singleton defs at the beginning or end of the body and hope nobody notices, and also update the screenshot plugin.
2. Designate one workspace to hold the singleton defs. We'd have to shuffle it around as workspaces are created and disposed, and we'd still have to update the screenshot plugin.
3. Leave everything the way it is and close this issue as "won't do".
4. Keep the defs in the workspace but use CSS variables to refer to them.

I think I'm leaning toward 4 now, if we're interested in using CSS to style SVG fills.

[BeksOmega]

iirc the reason I wanted to move it to be styled with CSS is b/c currently the places where the defs are used aren't overridable b/c they're assigned on elements' inline styles. So if you wanted to define your own custom def in an SVG outside of Blockly and apply it to blocks, you can't do it without using !important, which is annoying. (again iirc)

I think that using a css variable to refer to the defs solves this just as easily as the original proposal!

[johnnesky]

Still working on this, but there are some hiccups.

A workspace is actually multiple workspaces. There's the main block canvas, and then there's the toolbox flyouts. Both of these have their own defs and DOM hierarchy. There's also some temporary SVG containers, such as the blocklyBlockDragSurface which is not actually inside either of the above workspaces, but is intentionally rendered in front of both of them so that dragged objects appear in front of the flyout. This means that any CSS variables assigned to the root SVG group element of a workspace do not take effect for a block that is being dragged.

So we may need to unify the defs between the flyout and the block canvas, (which is a step in the direction of having singleton defs but would not unify with unrelated workspaces) and then assign the resulting CSS vars to the injectionDiv instead.