header clickable even if no data

[mathieupetrini]

Related to #612

[mathieupetrini]

@jbetancur Can you take a look to this PR ?

[jbetancur]

@mathieupetrini thanks for the PR. Sorry for the delay as I have been quite busy. So are you proposing to remove disabling the header and just disable to individual TableCols? Curious if there a specific use case that led to this?

[mathieupetrini]

The goal of this PR is to remove disabling the header when there is any data.

The purpose is to keep filter include in header available to be changed.

[jbetancur]

A few comments on naming. Also, we also need a snapshot test for this case scenario. Give the test a shot otherwise I can add it later.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@a3998ae). Click here to learn what that means.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master     #699   +/-   ##
=========================================
  Coverage          ?   99.38%           
=========================================
  Files             ?       43           
  Lines             ?      489           
  Branches          ?      137           
=========================================
  Hits              ?      486           
  Misses            ?        3           
  Partials          ?        0           

Impacted Files	Coverage Δ
src/DataTable/DataTable.js	100.00% <ø> (ø)
src/DataTable/TableHeadRow.js	100.00% <ø> (ø)
src/DataTable/TableCol.js	100.00% <100.00%> (ø)
src/DataTable/Pagination.js	100.00% <0.00%> (ø)
src/DataTable/ExpanderRow.js	100.00% <0.00%> (ø)
src/icons/Left.js	100.00% <0.00%> (ø)
src/DataTable/styles.js	100.00% <0.00%> (ø)
src/icons/FirstPage.js	100.00% <0.00%> (ø)
src/DataTable/TableColExpander.js	100.00% <0.00%> (ø)
src/DataTable/TableHead.js	100.00% <0.00%> (ø)
... and 36 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3998ae...a5aa1d8. Read the comment docs.

[mathieupetrini]

I had a snapshot test. I'm not sure about is quality if you can give a look.

Thanks for reviewing

[jbetancur]

Why is the name block is needed or funcitonHadtoBeClicked? You just need to mock someone attempting to click the cell when data is 0 or prrogresspending is true

[jbetancur]

these are the tests you need

  test('should not call onSort if progressPending true', () => {
    const onSortMock = jest.fn();
    const mock = dataMock({ sortable: true });
    const { container } = render(
      <DataTable
        data={mock.data}
        columns={mock.columns}
        onSort={onSortMock}
        persistTableHead
        progressPending
      />,
    );

    fireEvent.click(container.querySelector('div[id="column-some.name"]'));

    expect(onSortMock).not.toBeCalled();
  });

and

  test('should not call onSort if data is empty []', () => {
    const onSortMock = jest.fn();
    const mock = dataMock({ sortable: true });
    const { container } = render(
      <DataTable
        data={[]}
        columns={mock.columns}
        onSort={onSortMock}
        persistTableHead
      />,
    );

    fireEvent.click(container.querySelector('div[id="column-some.name"]'));

    expect(onSortMock).not.toBeCalled();
  });

[mathieupetrini]

No, i even want to be able to click on div or focus input in TableCol.

For example, here i cannot click on select under "Status" ...

[jonathanxlee]

Is there any update on this feature. This would be very helpful.

[mounicagdv]

how to make the columns header clickable for sorting

[abaum-augu]

Any updates on this? @jbetancur if you still interested on a use case for this, we are developing a multi table component to which you can pass single tables as child.

<MultiTable>
  <Table/>
  <Table/>
</MultiTable>

The Multi table has a header that controls sorting for all of the child tables. That header is actually an empty data table. So in this case the sorting buttons are disabled, therefore can't sort the children tables data.

[jbetancur]

/inactive