Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

header clickable even if no data #699

Closed
wants to merge 6 commits into from
Closed

header clickable even if no data #699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
96b14ac
remove disabled on tableheadrow when data is empty and move it to Tab…
mathieupetrini Oct 13, 2020
bb5e0c7
fix some style
mathieupetrini Oct 13, 2020
667209c
fix tu
mathieupetrini Oct 13, 2020
d7b3c4d
fix review.
mathieupetrini Nov 18, 2020
fe4023b
add snapshot test
mathieupetrini Nov 18, 2020
a5aa1d8
fix review
mathieupetrini Nov 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/DataTable/DataTable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,6 @@ const DataTable = memo(({
className="rdt_TableHeadRow"
role="row"
dense={dense}
disabled={progressPending || data.length === 0}
mathieupetrini marked this conversation as resolved.
Show resolved Hide resolved
>
{selectableRows && (
showSelectAll
Expand All @@ -349,6 +348,7 @@ const DataTable = memo(({
<TableCol
key={column.id}
column={column}
disabled={progressPending || data.length === 0}
sortIcon={sortIcon}
/>
))}
Expand Down
24 changes: 15 additions & 9 deletions src/DataTable/TableCol.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ const ColumnSortable = styled.div`
height: 100%;
line-height: 1;
user-select: none;
${props => (props.sortActive ? props.theme.headCells.activeSortStyle : props.theme.headCells.inactiveSortStyle)};
${props => (props.sortActive && props.sortable ? props.theme.headCells.activeSortStyle : props.theme.headCells.inactiveSortStyle)};

span.__rdt_custom_sort_icon__ {
i,
svg {
${props => (props.sortActive ? 'opacity: 1' : 'opacity: 0')};
${props => (props.sortActive && props.sortable ? 'opacity: 1' : 'opacity: 0')};
color: inherit;
font-size: 18px !important;
height: 18px !important;
Expand Down Expand Up @@ -51,6 +51,7 @@ const ColumnSortable = styled.div`

const TableCol = memo(({
mathieupetrini marked this conversation as resolved.
Show resolved Hide resolved
column,
disabled,
sortIcon,
}) => {
const { dispatch, pagination, paginationServer, sortColumn, sortDirection, sortServer, selectableRowsVisibleOnly, persistSelectedOnSort } = useTableContext();
Expand Down Expand Up @@ -120,18 +121,18 @@ const TableCol = memo(({
role="columnheader"
tabIndex={0}
className="rdt_TableCol_Sortable"
onClick={handleSortChange}
onKeyPress={handleKeyPress}
sortActive={sortActive}
onClick={!disabled ? handleSortChange : undefined}
onKeyPress={!disabled ? handleKeyPress : undefined}
sortActive={!disabled && sortActive}
column={column}
>
{customSortIconRight && renderCustomSortIcon()}
{nativeSortIconRight && renderNativeSortIcon(sortActive)}
{!disabled && customSortIconRight && renderCustomSortIcon()}
{!disabled && nativeSortIconRight && renderNativeSortIcon(sortActive)}
<div>
{column.name}
</div>
{customSortIconLeft && renderCustomSortIcon()}
{nativeSortIconLeft && renderNativeSortIcon(sortActive)}
{!disabled && customSortIconLeft && renderCustomSortIcon()}
{!disabled && nativeSortIconLeft && renderNativeSortIcon(sortActive)}
</ColumnSortable>
)}
</TableColStyle>
Expand All @@ -140,10 +141,15 @@ const TableCol = memo(({

TableCol.propTypes = {
column: PropTypes.object.isRequired,
disabled: PropTypes.bool,
sortIcon: PropTypes.oneOfType([
PropTypes.bool,
PropTypes.object,
]).isRequired,
};

TableCol.defaultProps = {
disabled: false,
};

export default TableCol;
7 changes: 1 addition & 6 deletions src/DataTable/TableHeadRow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
import styled, { css } from 'styled-components';

const disabled = css`
pointer-events: none;
`;
import styled from 'styled-components';

const TableHeadRow = styled.div`
display: flex;
align-items: stretch;
width: 100%;
${props => props.theme.headRow.style};
${props => (props.dense && props.theme.headRow.denseStyle)};
${props => props.disabled && disabled};
`;

export default TableHeadRow;
25 changes: 25 additions & 0 deletions src/DataTable/__tests__/DataTable.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,31 @@ describe('DataTable::progress/nodata', () => {

expect(container.firstChild).toMatchSnapshot();
});

test('should render correctly when persistTableHead and no data', () => {
const functionHadToBeClicked = jest.fn(() => true);
const onSortMock = jest.fn();

const mock = dataMock({
// eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events
name: (<div id="testColName" onClick={functionHadToBeClicked}>Test</div>),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Owner

@jbetancur jbetancur Nov 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();
  });

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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" ...
image

sortable: false,
});

const { container } = render(
<DataTable
data={[]}
columns={mock.columns}
onSortMock={onSortMock}
persistTableHead
selectableRows
/>,
);

fireEvent.click(container.querySelector('#testColName'));
expect(functionHadToBeClicked.mock.calls.length).toBe(1);
expect(onSortMock.mock.calls.length).toBe(0);
});
});

describe('when noTableHead', () => {
Expand Down