[DataGrid] Emit onSelectionModelChange when selection state changes

[DanailH]

Fixes #1782

[oliviertassinari]

👍 for this behavior, it also fixes the uncontrolled behavior.

[oliviertassinari]

Looks good

[dtassone]

not sure how this fix is going to fit with my control state PR, it seems redundant, and we will have to clean these changes

[DanailH]

@dtassone It's not a fix per se but rather a missed interaction that should emit the onSelectionModelChange.

[dtassone]

I think it will go see https://github.com/mui-org/material-ui-x/pull/1823/files#diff-87fa96c5d959b8acbc3c76f30e1750e8293f462996dd1a1e8cba8e72553d2f5a

[DanailH]

Well if that is the case then why is the linked ticket assigned to me, we are essentially doing double work here...

[oliviertassinari]

The PR looks solid, I would personally 👍 for a merge: to encourage moving one step at a time. The rebase in #1823 will likely be easy, and if the logic is correct, the test will still pass. If the test doesn't pass it should likely be #1823 responsibility to fix it. I would actually hope that an extra constraint from here will help #1823 tailor the solution to reality.

[DanailH]

@oliviertassinari @dtassone Ok, I applied the test to the control state PR #1823. This is what the test should look like taking into account the changes in the linked PR. The only part that is failing is the part of clearing the selection after loading rows with different IDs.

it('should call onSelectionModelChange when selection state changes', () => {
    const handleSelectionModelChange = spy();
    const { setProps } = render(
      <div style={{ width: 300, height: 300 }}>
        <XGrid 
          columns={[{ field: 'brand' }]}
          rows={[
            {
              id: 0,
              brand: 'Nike',
            },
          ]}
          selectionModel={[0]} 
          onSelectionModelChange={handleSelectionModelChange} />
      </div>
    );
    expect(getSelectedRows(apiRef)).to.deep.equal([0]);
    setProps({
      rows: [
        {
          id: 1,
          brand: 'Nike',
        },
      ],
    });
    expect(handleSelectionModelChange.callCount).to.equal(1);
    expect(handleSelectionModelChange.getCall(0).args[0]).to.deep.equal([]);
    // TODO: Fix - it should clear the selection because the newly loaded rows have different IDs
    expect(getSelectedRows(apiRef)).to.deep.equal([]);
  });

I can either push this directly to the control state PR or @dtassone you can add it manually.

[dtassone]

IMO we could merge #1823 and then reapply/fix this issue on top of it

[oliviertassinari]

Great 👍