DataTable header checkbox not updated after programmatically unchecking all items

pboguslawski commented 1 year ago

Describe the bug DataTable header checkbox state is not updated after programmatically (un)checking items using selected array.

To Reproduce

Steps to reproduce the behavior:

Create component with "Row selection" source from https://sveltematerialui.com/demo/data-table/

Append lines

<a on:click={() => (selected.length = 0)}>Uncheck all</a>
<a on:click={() => (selected = [options[0], options[1], options[2], options[3], options[4]])}>Check all</a>
<a on:click={() => (selected = [options[0], options[1], options[2], options[3]])}>Check some</a>

Select all items using checkbox in table header (to make header checkbox checked); optionally uncheck a few items leaving w few checked (to leave header checkbox as "-").
Click "Unselect all" link.

Expected behavior Header checkbox should be unchecked if was checked or "-".

Similar problem when unchecking all using header checkbox and then pressing "Check all" or "Check some" link.

Desktop:

OS: linux
Browser any

vhscom commented 1 year ago

Related to https://github.com/hperrin/svelte-material-ui/issues/240, in which the select all checkbox becomes checked when a user selects an extraneous checkbox located within a data table.

vhscom commented 1 year ago

Minimal reproduction: https://github.com/vhscom/smui-issue-562-repo-data-table-header-checkbox

Verified issue is not a clear regression by backing down from ^7.0.0-beta.0 to v6.2.0 and can reproduce.

I've created two workarounds, one using a Svelte key block here: https://github.com/vhscom/smui-issue-562-repo-data-table-header-checkbox/tree/key-block-workaround

And another workaround by controlling the state of the header checkbox manually: https://github.com/vhscom/smui-issue-562-repo-data-table-header-checkbox/tree/state-handling-workaround

Video of key workaround in action attached (same effect as the state management workaround):

https://user-images.githubusercontent.com/97140109/217384097-a9eed530-e69a-4fad-940c-a2fbd7b2bf62.mp4

In reviewing the upstream MDC-Web component framework I did not see any related issues so I assume this a SMUI bug and the DataTable component notifies of row selection change when bound collection mutated.

campbellcole commented 1 year ago

I have also discovered that removing items from the table while some are selected causes completely unpredictable (to me at least) behavior. I have a basic table with CRUD support, and removing rows completely breaks the selection. Also, as demonstrated in the above video, the rows stay in the selected state even after their checkbox has become unchecked. I peeked into the code and saw there's some weird state management going on and I can't pretend I understand it, but it's clearly not working properly. The most reliable workaround I've come up with is programmatically clicking the uppermost checkbox once or twice depending on how many elements were selected, as this completely propagates the events as intended.

campbellcole commented 1 year ago

This workaround I have just come up with solves all of these problems for now. I think it's very strange that we have to do this but oh well, it works:

<script lang="ts">
... (imports and such) ...
let mainCheckboxInst: Checkbox;
let boxes: {
  [index: number]: Checkbox
} = {};
let selected = [];

const deselectAll = () => {
  if (selected.length == 0) {
    return;
  }
  if (selected.length != Object.keys(boxes).length) {
    mainCheckboxInst.getElement().querySelector("input")!.click();
  }
  mainCheckboxInst.getElement().querySelector("input")!.click();
};

const deleteSelected = async () => {
  const ids = selected.map((sel) => sel.id);
  for (let id of ids) {
    boxes[id].getElement().querySelector("input")!.click();
  }
  // ... actual fetch request or something here
  options = options.filter((option) => !ids.includes(option.id));
  deselectAll();
};
</script>

<DataTable stickyHeader>
  <Head>
    <Row>
      <Cell checkbox><Checkbox bind:this={mainCheckboxInst} /></Cell>
      <Cell>(option fields here)</Cell>
      ...
    </Row>
  </Head>
  <Body>
    {#each options as option}
      <Row>
        <Cell checkbox>
          <Checkbox
            bind:group={selected}
            bind:this={boxes[option.id]}
            value={option}
            valueKey={''+option.id}
          />
        </Cell>
        <Cell>{... (option fields here) ...}</Cell>
        ...
      </Row>
    {/each}
  </Body>
</DataTable>

The important parts here are: 1) make sure to click the header checkbox twice if we haven't selected every element, because the first click will select the rest 2) if you remove a field, BEFORE REMOVING IT, you must click the corresponding checkbox to remove it from the selections group, otherwise the entire thing falls to pieces and completely fails to manage the state properly from that point on

My takeaway from this is that the entire checkbox state management seems to be built upon click events and not state changes to the checked prop, because that is abstracted one step away from the actual checkbox through some strange magic that goes way over my head. Before arriving here, I tried every combination of managing props manually, and literally nothing worked at all until I started sending click events to control the state. Hope this can be fixed soon because I think this is very inefficient.

hperrin / svelte-material-ui

DataTable header checkbox not updated after programmatically unchecking all items #562