[calcite-table,calcite-list] Refactor component to improve compatibility with lit-html

[maxpatiiuk]

Description

Post-codemod, most Calcite components work with no or minimal changes.
However, <calcite-table> and <calcite-list> may require some changes because of the way lit-html works.

lit-html converts each template to a <template> element behind the scenes. That means each template needs to be standalone piece of valid HTML.

That is causing issues in <calcite-table> - since the table is rendered in pieces, some templates are not valid standalone (tables have strict rules about what elements can be inside of them), and so the browser moves the "invalid" elements out of the table.
Similar issues are present in <calcite-list> since it's rendered as a table underneath.

Code sample (take a look in devtools at what does the rendered html look like)

The solution proposed by Lit is to use div[role="cell"] and etc instead of td inside the calcite-table components.
then with css apply display:table-cell;
Example: https://lit.dev/playground/#gist=0b495facaf08c1f990e9879f54405fc8

The other options might be to rendering more parts of the table as a single lit-html template, or doing imperative rendering in table components.

As far as I am aware, other component are not subject to this issue - it's just table that has these restrictions.

Proposed Advantages

calcite-table needs to be refactored a bit to render correctly in lit-html.
Until then, the browser may move what it sees as "HTML element with incorrect placement" out of the table, breaking the layout:

Which Component

calcite-table

Relevant Info

No response

Calcite package

• @esri/calcite-components
• @esri/calcite-components-angular
• @esri/calcite-components-react
• @esri/calcite-design-tokens
• @esri/eslint-plugin-calcite-components

[macandcheese]

#6646 will provide imperative rendering, but I know there is value for a lot of our "dev summit users" / simple use cases to stand up a static table declaratively. If that can be preserved in some way, that would be nice for those cases - alongside the more advanced functionality of course.

[maxpatiiuk]

If that can be preserved in some way, that would be nice for those cases

Agree. More research needs to be done, but it appears that the required changes would be done inside the calcite-table code, rather than affecting outside usage.

e.g. if it worked in Stencil, it should work in Lit. The only thing that changed is the rendering engine used by calcite-table.

[driskull]

Is this an issue with calcite-list? It places divs around <tr> etc.

[maxpatiiuk]

Is this an issue with calcite-list? It places divs around etc.

You are correct, that appears to be the case. I did not realize calcite-list was using a table underneath.
I will edit the issue title to mention both.

[driskull]

so the browser moves the "invalid" elements out of the table.

Is this really the case? Wouldn't this be occurring now with these components? This seems like some kind of lit thing not a browser thing.

Reading the discussion it sounds like it is a lit thing that its doing because of web spec rules. So its likely something we should fix but it does complicate the conversion to lit.

[driskull]

I think it may be more than just converting to a div with a role. The colSpan/rowSpan styling might be challenging here.

I'd also be concerned with how this affects a11y. If a11y does have an issue with a component between a tr/td then maybe its worth it. Otherwise its a lot to work around.

[macandcheese]

I tried the role approach and pushed an example to macandcheese/lit-table-demo branch if you want to look - I see a similar "wrong" result visually, though I could have missed a couple styles in the changes. You do of course lose "native" colSpan / rowSpan support as mentioned.

Edit - oops, of course this is the Stencil version, so you'd need to run the codemod on that branch to see the "true" result I suppose.

[driskull]

@maxpatiiuk does it also have an issue with <li> elements that are not directly within a <ul>/<ol>? Because we have some occurrences of that as well.

[maxpatiiuk]

does it also have an issue with <li> elements...

I do not know. I opened this issue so that someone from the Calcite team could do the research about the exact cause of the problem and the possible solutions.

Is this really the case? Wouldn't this be occurring now with these components? This seems like some kind of lit thing not a browser thing.

The main difference is how functional components are parsed & rendered:

• In calcite-table you have render() that calls renderTHead() and renderTBody().
• In Stencil, the JSX of these 3 methods is combined into one, and then Calcite walks the JSX tree to convert JSX to actual DOM elements
• In lit-html, each functional component is a separate html``, that is converted to a separate HTML <template> behind the scenes, and that each <template> is parsed by the browser independently. Afterward, it will use template. cloneNode and append the dom fragments together
  • If this is the core of the issue, then maybe it's just about collapsing multiple functional components inside calcite-table into one
    • The other option is to imperatively create the thead/tbody elements in calcite-table and calcite-list, rather than using jsx
  • Otherwise, if the issue is about the divs in between, then that I ask you to research why this wasn't an issue in Stencil and how we can workaround.

[maxpatiiuk]

Edit - oops, of course this is the Stencil version, so you'd need to run the codemod on that branch to see the "true" result I suppose.

I have a Lumina branch in a PR - #10482 - feel free to experiment on top of it:

But you can also create a small lit app (not even lumina) and play around with how it renders a table - see for example this Lit playground "Code sample"

[maxpatiiuk]

Lit also has static templates where you can create template from dynamic parts - https://lit.dev/docs/templates/expressions/#static-expressions

You could have renderTHead return an HTML string, that is inlined inside the render() method's template.
Not saying this is ideal DX, but there are options so need to investigate them

[driskull]

If this is the core of the issue, then maybe it's just about collapsing multiple functional components inside calcite-table into one

I think this is the issue but this wouldn't work since the table rows/cells are slotted in, so combining them with the table is not a viable option.

the issue is about the divs in between, then that I ask you to research why this wasn't an issue in Stencil and how we can workaround.

This is also an issue since HTML table requires certain direct children. However, stencil doesn't stop you from doing this and it doesn't seem like screen readers care if the HTML isn't valid. It seems like lit does stop you potentially because of the HTML template being used.

The solution seems to be to use aria roles for our use cases.

[macandcheese]

I checked out the Lumina branch - it seems like the Table renders correctly in some cases (there are some styling and keyboard navigation things here and there, but the structure / layout is accurate).

However the cases that appear broken (as shown in screenshot in original post) - we are programmatically rendering additional cells on behalf of the user - this occurs for numbered and when selectionMode is set. Maybe we can play with the new lifecycle methods and see if rendering these at a different time / in a different matter works.

[driskull]

I have a PR open to fix the list component.

We may need to identify other components that may be affected with ul > li where the li isn't a direct child of ul.

This is one of those quirks with web components where the parent/child can't be separated by a shadow DOM.

[driskull]

I think the following components will need updating as well:

• combobox/combobox-item
• menu/menu-item

[driskull]

@jcfranco do we want to update this issue to include combobox and menu? I think those are the only ones using ol > li inappropriately.

[maxpatiiuk]

It might not be necessary in menu/combobox. All menu E2E tests are passing on the Lumina branch without refactoring render(). Some combobox tests are failing but the error messages don't look related to this.

On the other hand, List and Table tests are failing due to html elements being moved out.

[driskull]

Ok maybe tables are the only ones with immediate issues. We should have a followup issue to refactor the ul > li ones post 3.0

[driskull]

List PR has been installed so it should be good now.

[github-actions]

Installed and assigned for verification.

[DitwanP]

🍡 Verified