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.

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

Add column.colSpan prop #2356

Merged
merged 57 commits into from
Apr 21, 2021
Merged

Add column.colSpan prop #2356

merged 57 commits into from
Apr 21, 2021

Conversation

amanmahajan7
Copy link
Contributor

@amanmahajan7 amanmahajan7 commented Apr 13, 2021

colSpan?: (args: ColSpanArgs<TRow, TSummaryRow>) => number | undefined;

export type ColSpanArgs<R, SR> = {
  type: 'HEADER' | 'FILTER';
} | {
  type: 'ROW';
  row: R;
} | {
  type: 'SUMMARY';
  row: SR;
};

It can be used as

colSpan(args) {
  if (args.type === 'ROW') {
    if (key === '2' && args.row === 2) return 3;
    if (key === '4' && args.row === 4) return 6; // Will not work as colspan includes both frozen and regular columns
    if (key === '0' && args.row === 5) return 5;
    if (key === '27' && args.row === 8) return 3;
    if (key === '6' && args.row < 8) return 2;
  }
  if (args.type === 'HEADER' && key === '8') {
    return 3;
  }
  return undefined;
},

@ekeuus
Copy link
Contributor

ekeuus commented Apr 13, 2021

Why not go for a non-manually calculated api? #2350

@amanmahajan7
Copy link
Contributor Author

@ekeuus colSpan is a generic api that can handle any type of row so not just header. We can even have both apis if makes sense. Thank your for the header groups PR. We will review it soon

@@ -279,6 +289,14 @@ function DataGrid<R, SR>({
enableVirtualization
});

const viewportColumns = useViewportColumns({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to split into two hooks as we need to calculate colspan for all the rows in the viewport. This is needed to ensure a column that spans a column in the viewport is also visible

return (
<div
role="row"
aria-rowindex={1} // aria-rowindex is 1 based
className={headerRowClassname}
>
{columns.map(column => {
if (colSpan && colSpan > 1) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do not render cells in the colspan range

@@ -0,0 +1,252 @@
import { useMemo } from 'react';
Copy link
Contributor Author

@amanmahajan7 amanmahajan7 Apr 16, 2021

Choose a reason for hiding this comment

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

Same as previous useViewportColumns hook. I moved the last useMemo to the new useViewportColumns hook as it needs rows

return colSpan;
}

function areColSpanColumnsCompatible<R, SR>(column: CalculatedColumn<R, SR>, viewportColumns: readonly CalculatedColumn<R, SR>[], colSpan: number) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ensure all the columns within the colspan range are either frozen or not

@amanmahajan7 amanmahajan7 marked this pull request as ready for review April 19, 2021 21:33
src/HeaderRow.tsx Outdated Show resolved Hide resolved
src/HeaderRow.tsx Outdated Show resolved Hide resolved
src/Row.tsx Outdated Show resolved Hide resolved
src/Row.tsx Outdated Show resolved Hide resolved
src/SummaryRow.tsx Outdated Show resolved Hide resolved
src/utils/index.ts Outdated Show resolved Hide resolved
src/utils/index.ts Outdated Show resolved Hide resolved
src/utils/index.ts Outdated Show resolved Hide resolved
stories/demos/CommonFeatures.tsx Outdated Show resolved Hide resolved
src/utils/index.ts Outdated Show resolved Hide resolved
src/Row.tsx Outdated Show resolved Hide resolved
src/Row.tsx Show resolved Hide resolved
src/hooks/useViewportColumns.ts Outdated Show resolved Hide resolved
src/hooks/useViewportColumns.ts Outdated Show resolved Hide resolved
src/utils/index.ts Outdated Show resolved Hide resolved
test/column/colSpan.test.ts Outdated Show resolved Hide resolved
test/column/colSpan.test.ts Outdated Show resolved Hide resolved
test/column/colSpan.test.ts Outdated Show resolved Hide resolved
test/column/colSpan.test.ts Outdated Show resolved Hide resolved
src/utils/index.ts Outdated Show resolved Hide resolved
nstepien
nstepien previously approved these changes Apr 21, 2021
src/utils/selectedCellUtils.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@nstepien nstepien left a comment

Choose a reason for hiding this comment

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

:shipit:
Maybe now is a good time to update the PR title

@amanmahajan7 amanmahajan7 changed the title POC: ColSpan Add ColSpan column prop Apr 21, 2021
@amanmahajan7 amanmahajan7 changed the title Add ColSpan column prop Add column.colSpan prop Apr 21, 2021
@amanmahajan7 amanmahajan7 merged commit 6b88e0c into canary Apr 21, 2021
@amanmahajan7 amanmahajan7 deleted the am-col-span branch April 21, 2021 21:07
gernotkogler pushed a commit to Garaio-REM/react-data-grid that referenced this pull request May 13, 2021
* Add column.colSpan prop

* Change colSpan signature

* Handle keyboard navigation

* Add comments

* Update example

* Handle frozen columns

* Handle header and summary rows

* Update example

* Update viewport columns to handle colspan

* Add ColumnSpanning example

* Cleanup cell selection logic

* Update src/utils/index.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Update src/utils/index.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Remove unused import

* Check integer and non negative values

* Move to cell selection logic to DataGrid

* Update src/utils/index.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Tweak colors

* Fix eslint warning

* Split into two useMemos, fix startIdx

* Check header and summary rows

* Fix next position logic

* Make sure cell is within bounds

* Loop through columns once

* Do not close flighting editor on enter/escape

* Revert "Do not close flighting editor on enter/escape"

This reverts commit fc6ed3d.

* Add cellSpan tests

* Remove colSpan number type

* Optimize colSpan check

* Test navigation

* Fix types

* Check filter row

* Fix types

* Update src/hooks/useCalculatedColumns.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Update src/hooks/useViewportColumns.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Update src/hooks/useViewportColumns.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Update src/types.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Update src/hooks/useViewportColumns.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Update src/hooks/useViewportColumns.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Update src/hooks/useViewportColumns.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Quit loop when first startIdx is found

* Re-export ColSpanArgs

* Use lastFrozenColumnIndex to validate colSpan

* Update src/hooks/useViewportColumns.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Update src/utils/index.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Update test/column/colSpan.test.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Update test/column/colSpan.test.ts

Co-authored-by: Nicolas Stepien <[email protected]>

* Add getCellsAtRowIndex util

* Move nextPosition calculation logic to getNextSelectedCellPosition

* column.colSpan can't be a number

* Eslint

* Typecheck

* Reuse colIdx, add posIdx

* Move posIdx outside

* Add failing tests

* Fix cell navigation

Co-authored-by: Nicolas Stepien <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants