refactor(table): refactor internals to work around HTML parser issue #10649

jcfranco · 2024-10-30T05:52:49Z

Related Issue: #10495

Summary

Split up table rendering to bypass HTML parser corrections that would break functionality and styling.

packages/calcite-components/src/components/table-row/table-row.tsx

jcfranco · 2024-10-30T05:58:35Z

packages/calcite-components/src/components/table-row/table-row.tsx

@@ -410,11 +410,27 @@ export class TableRow extends LitElement implements InteractiveComponent {
          ariaSelected={this.selected}
          class={{ [CSS.lastVisibleRow]: this.lastVisibleRow }}
          onKeyDown={this.keyDownHandler}
-          ref={this.tableRowEl}
+          ref={(el) => {
+            if (!el || this.tableRowEl) {


TODO: improve how internal structure is updated, cleaned up

jcfranco · 2024-10-30T05:59:28Z

packages/calcite-components/src/components/table/table.tsx

@@ -108,7 +108,7 @@ export class Table extends LitElement implements LoadableComponent {
   * @notPublic
   */
  /** TODO: [MIGRATION] This component has been updated to use the useT9n() controller. Documentation: https://qawebgis.esri.com/arcgis-components/?path=/docs/references-t9n-for-components--docs */
-  messages = useT9n<typeof T9nStrings>();
+  messages = useT9n<typeof T9nStrings>({ blocking: true });


TODO: need to investigate why blocking the table's messages helps tests succeed consistently (possibly extra delay helps?)

jcfranco · 2024-10-30T06:01:57Z

packages/calcite-components/src/components/table/table.tsx

@@ -127,7 +127,11 @@ export class Table extends LitElement implements LoadableComponent {
   *
   * @readonly
   */
-  @property() selectedItems: TableRow["el"][] = [];
+  @property() get selectedItems(): TableRow["el"][] {


TODO: clean up

jcfranco · 2024-10-30T06:02:58Z

packages/calcite-components/src/components/table/table.tsx

            ariaRowCount={this.allRows?.length}
            class={{ [CSS.tableFixed]: this.layout === "fixed" }}
+            ref={(el) => {
+              if (!el || this.tableHeadSlotEl) {


TODO: tidy up, improve naming

packages/calcite-components/src/components/table/table.tsx

jcfranco · 2024-10-30T06:05:43Z

packages/calcite-components/src/components/table-header/table-header.tsx

@@ -74,7 +74,7 @@ export class TableHeader extends LitElement implements LoadableComponent {
   * @notPublic
   */
  /** TODO: [MIGRATION] This component has been updated to use the useT9n() controller. Documentation: https://qawebgis.esri.com/arcgis-components/?path=/docs/references-t9n-for-components--docs */
-  messages = useT9n<typeof T9nStrings>();
+  messages = useT9n<typeof T9nStrings>({ blocking: true });


Needed to block these to ensure an a11y test passes (https://dequeuniversity.com/rules/axe/4.10/empty-table-header?application=axeAPI).

packages/calcite-components/src/components/table/table.tsx

packages/calcite-components/src/components/table-row/table-row.tsx

maxpatiiuk · 2024-10-31T18:07:56Z

packages/calcite-components/src/components/table-row/table-row.tsx

-      />
-    );
+  private renderSelectableCell(): void {
+    const cell: any =


(optional) store the reference to the element that was created on previous render and update it rather than create a new element?
that would improve re-render performance

maxpatiiuk · 2024-10-31T18:08:00Z

packages/calcite-components/src/components/table-row/table-row.tsx

-      />
-    );
+  private renderNumberedCell(): void {
+    const cell =


(optional) store the reference to the element that was created on previous render and update it rather than create a new element?
that would improve re-render performance

packages/calcite-components/src/components/table/table.e2e.ts

packages/calcite-components/src/components/table/table.tsx

packages/calcite-components/src/components/table-row/table-row.tsx

maxpatiiuk · 2024-10-31T18:33:01Z

Did you find out what was the main issue with rendering the table in parts with JSX?
was is that each lit-html template is parsed independently and so is not valid unless taken wholistically?
or that lit-html doesn't correctly append the elements to the table element? (appends them, but browser moves them out?)

depending on what was the cause, you might be able to make this PR a bit more declarative by directly calling lit-html's render() function to render desired content into the desired container. I have an example of this in the docs: https://qawebgis.esri.com/arcgis-components/?path=/docs/lumina-jsx--docs#rendering-jsx-outside-the-component

render() will also take care of updating the previously rendered element in place if you call it again with the same markup

a limitation though: you can only render one template into some parent container at a given time using render() - if you try to render() a different template into the same container, the 2nd render() call will overwrite the result of the 1st call. this might be a deal breaker since you need to render thead, tbody and tfoot into the same parent

jcfranco · 2024-11-02T00:00:14Z

Did you find out what was the main issue with rendering the table in parts with JSX?
was is that each lit-html template is parsed independently and so is not valid unless taken wholistically?
or that lit-html doesn't correctly append the elements to the table element? (appends them, but browser moves them out?)

My guess is that the final template was processed and adjusted by the HTML parser, whereas dynamically creating the table structure through DOM APIs bypasses the parser.

…10649) **Related Issue:** #10495 ## Summary Split up table rendering to bypass HTML parser corrections that would break functionality and styling.

refactor(table): refactor internals to work around HTML parser issue

2508f23

jcfranco marked this pull request as draft October 30, 2024 05:53

remove test delay

e7030dd

jcfranco commented Oct 30, 2024

View reviewed changes

jcfranco assigned jcfranco and macandcheese Oct 30, 2024

macandcheese reviewed Oct 30, 2024

View reviewed changes

packages/calcite-components/src/components/table/table.tsx Show resolved Hide resolved

maxpatiiuk reviewed Oct 31, 2024

View reviewed changes

jcfranco added 5 commits October 31, 2024 12:18

restore aria attr

b5f395a

tidy up

e09d991

avoid listenOn in connectedCallback

add8304

restore reactive rendering by using separate render

a31fe3e

fix lit-html event handling error

147ed3b

tidy up

a6bf5b5

jcfranco marked this pull request as ready for review November 3, 2024 23:59

jcfranco merged commit f629a5f into lumina Nov 4, 2024
5 checks passed

jcfranco deleted the jcfranco/refactor-table-to-workaround-html-parser-issue branch November 4, 2024 00:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(table): refactor internals to work around HTML parser issue #10649

refactor(table): refactor internals to work around HTML parser issue #10649

jcfranco commented Oct 30, 2024 •

edited

Loading

jcfranco Oct 30, 2024

jcfranco Oct 30, 2024

jcfranco Oct 30, 2024

jcfranco Oct 30, 2024

jcfranco Oct 30, 2024

maxpatiiuk Oct 31, 2024

maxpatiiuk Oct 31, 2024

maxpatiiuk commented Oct 31, 2024

jcfranco commented Nov 2, 2024 •

edited

Loading

refactor(table): refactor internals to work around HTML parser issue #10649

refactor(table): refactor internals to work around HTML parser issue #10649

Conversation

jcfranco commented Oct 30, 2024 • edited Loading

Summary

jcfranco Oct 30, 2024

Choose a reason for hiding this comment

jcfranco Oct 30, 2024

Choose a reason for hiding this comment

jcfranco Oct 30, 2024

Choose a reason for hiding this comment

jcfranco Oct 30, 2024

Choose a reason for hiding this comment

jcfranco Oct 30, 2024

Choose a reason for hiding this comment

maxpatiiuk Oct 31, 2024

Choose a reason for hiding this comment

maxpatiiuk Oct 31, 2024

Choose a reason for hiding this comment

maxpatiiuk commented Oct 31, 2024

jcfranco commented Nov 2, 2024 • edited Loading

jcfranco commented Oct 30, 2024 •

edited

Loading

jcfranco commented Nov 2, 2024 •

edited

Loading