Render table in the shadow DOM for @finos/perspective-viewer-datagrid

[tomjakubowski]

Move the <regular-table> element of datagrid into the shadow DOM by default.

This change isolates the regular-table from global styles, so CSS rules on regular-table or table no longer affect datagrid. Style rules on the regular-table can be ported using the part named regular-table

// old style
perspective-viewer-datagrid regular-table {
  font-family: "Comic Sans MS";
}
// shadow dom
perspective-viewer-datagrid::part(regular-table) {
  font-family: "Comic Sans MS";
}

To disable this feature and continue to use light DOM rendering, set:

HTMLPerspectiveViewerDatagridPluginElement.renderTarget = "light"

[texodus]

Thanks for the PR! A couple of things need fixing:

1. I'm ok with the configuration flag being a global static on the perspective-viewer-datagrid element, because there is no global settings feature yet, but one is obviously needed. However, let's please rename this to enableShadowDOM.
2. enableShadowDOM should be true by default, e.g. we should default use the Shadow DOM. It looks like the examples work in this mode with this PR (👍 ), so no changes needed there. It is ok if the tests remain as Light DOM tests because they do some DOM surgery to check table elements, but ...
3. There needs to be at least a few tests added that test Shadow DOM mode specifically. If it's doable, I'd really like to see all the tests run in parameterized in both modes, but this may be a lot of work (?)
4. The tests are currently failing regardless (looks like perspective-jupyterlab).
5. Let's remove the examples/blocks code which parses the URL.

[tomjakubowski]

I'm ok with the configuration flag being a global static on the perspective-viewer-datagrid element, because there is no global settings feature yet, but one is obviously needed. However, let's please rename this to enableShadowDOM.

Another API I was thinking about was sticking it as an attribute/flag on <perspective-viewer>, like <perspective-viewer shadow />. A problem is I imagine it would be disastrous to update a particular element to use/not use shadow DOM at runtime, i.e. to toggle usage of shadow DOM in response to changes to that attribute.

There needs to be at least a few tests added that test Shadow DOM mode specifically. If it's doable, I'd really like to see all the tests run in parameterized in both modes, but this may be a lot of work (?)

I'll try this, it's a good idea to rerun the same tests with shadow DOM enabled/disabled. And I'll think about a test or two just for shadow DOM

[tomjakubowski]

All the checks are passing now, re-review?

[tomjakubowski]

curious if you have any recommendations for quickly diffing these tarballs?

[texodus]

Not sure what you mean. In theory, if the snapshots don't change, the tarball should be identical. We've added a few flags to the gzip process to force this to be the case regardless of things like file creation time.

I have noticed that this is not always the case, but I don't know why yet, nor have I even validated that this is an issue with the tarball or the actual files.

[texodus]

Thanks for the PR! Looks good!

[jakehadar]

I'm ok with the configuration flag being a global static on the perspective-viewer-datagrid element, because there is no global settings feature yet, but one is obviously needed. However, let's please rename this to enableShadowDOM.

Another API I was thinking about was sticking it as an attribute/flag on <perspective-viewer>, like <perspective-viewer shadow />. A problem is I imagine it would be disastrous to update a particular element to use/not use shadow DOM at runtime, i.e. to toggle usage of shadow DOM in response to changes to that attribute.

There needs to be at least a few tests added that test Shadow DOM mode specifically. If it's doable, I'd really like to see all the tests run in parameterized in both modes, but this may be a lot of work (?)

I'll try this, it's a good idea to rerun the same tests with shadow DOM enabled/disabled. And I'll think about a test or two just for shadow DOM

Hi, was this attribute flag ever exposed publicly? Looks like it is a static/private attribute set at runtime based on browser capaibility https://github.com/finos/perspective/blame/84605851230c3dd1cc58652b47babd04cc9fcc31/packages/perspective-viewer-datagrid/src/js/custom_elements/datagrid.js#L141
This introduces inconsistencies between browsers and i believe the shadow rendering policy should be something the consumer should have the option to opt-out of

[tomjakubowski]

@jakehadar The inferred setting is a default. You can override it by using customElements.whenDefined() to access the custom element constructor when it is added to the registry, and then set the renderTarget property on it to whichever you prefer.

customElements.whenDefined("perspective-viewer-datagrid").then((datagrid) => {
  console.log("setting datagrid render target");
  datagrid.renderTarget = "light";
});

datagrid.renderTarget is currently an undocumented API, but I don't anticipate it will change in the near term.

[jakehadar]

Awesome, this is exactly what I need. Thank you for the quick response!