Improve Cell navigation Performance #1123

malonecj · 2018-02-12T14:29:25Z

Description

This is an attempt to see if its possible to perform cell navigation with keyboard at 60FPS.
Rather than the Cell component being responsible for displaying a selected style, a new SelectionMask component which sits at Canvas level, is responsible for displaying the selected cell.

Results

The proof of concept was successful, showing that every frame is rendered at over 60FPS.

Tasks

Bugs

Cell double click editing does not work
Clicking on a cell scrolls the canvas to the top
Fix SelectedCell position in the grouping example
Fix zIndex for filters

- Remove extra ref - Format JSX

Delete/Ignore package-lock.json

Delete package.json for now

- Fix formatting - Remove extra spread operator

Remove unused methods

- Add basic implementation for store/reducer/dispatch - Remove CellContainer to prevent rerendering of each cell on selection change

Cleanup event handlers

Check if cell is editable

Remove commented code

amanmahajan7 · 2018-05-01T12:26:50Z

packages/react-data-grid-addons/src/__tests__/data/MockStateObject.js

-      rowIdx: 0,
-      idx: 0
-    },
-    copied: null,


Cell selection/copy/drag logic is moved to the InteractionMasks component

amanmahajan7 · 2018-05-01T12:27:55Z

packages/react-data-grid-examples/src/index.html

@@ -21,11 +21,12 @@
  <link rel="stylesheet" href="assets/css/examples.css">
  <link rel="stylesheet" href="assets/css/react-context-menu.css">

+  <script src="https://cdnjs.cloudflare.com/ajax/libs/core-js/2.5.5/core.min.js"></script>


https://reactjs.org/blog/2017/09/26/react-v16.0.html#javascript-environment-requirements

amanmahajan7 · 2018-05-01T12:28:52Z

packages/react-data-grid/src/Canvas.js

+
+    const { scrollToRowIndex } = this.props;
+    if (prevProps.scrollToRowIndex !== scrollToRowIndex && scrollToRowIndex !== 0) {
+      this.scrollToRow(scrollToRowIndex);


Fixes #1084

amanmahajan7 · 2018-05-01T12:38:13Z

packages/react-data-grid/src/Canvas.js

+    if (this.canvas) {
+      this.canvas.scrollTop = scrollTop;
+      this.canvas.scrollLeft = scrollLeft;
+    }


Grid is automatically scrolled to the top when a cell is clicked, this is a workaround for that

amanmahajan7 · 2018-05-01T12:46:41Z

packages/react-data-grid/src/Grid.js

+  //     canvas.scrollLeft = scrollLeft;
+  //     this.viewport.canvas.setScrollLeft(scrollLeft);
+  //   }
+  // };


I am not sure if I should delete it or not

amanmahajan7 · 2018-05-01T12:48:00Z

packages/react-data-grid/src/ReactDataGrid.js

    if (this.props.onColumnResize) {
      this.props.onColumnResize(index, width);
    }
  };

-  onKeyDown = (e: SyntheticKeyboardEvent) => {
-    if (isCtrlKeyHeldDown(e)) {


All the selection logic is moved to the InteractionMasks component

amanmahajan7 · 2018-05-01T13:46:58Z

packages/react-data-grid/src/masks/EventBus.js

@@ -0,0 +1,28 @@
+class EventBus {


Cell component uses event bus to interact with the InteractionMasks component. The idea is to remove the local state from the ReactDataGrid component so that the whole tree is not rendered on every keystroke.

Click Cell

Fire SelectCell event

InteractionMasks is subscribed to the event and handles the selection logic

diogofcunha

Didn't do any deep dive on the code but the general idea looks fine to me.

diogofcunha · 2018-05-21T08:37:20Z

packages/react-data-grid/src/RowGroup.js

@@ -67,7 +72,8 @@ RowGroup.propTypes = {
  isExpanded: PropTypes.bool.isRequired,
  treeDepth: PropTypes.number.isRequired,
  name: PropTypes.string.isRequired,
-  renderer: PropTypes.func
+  renderer: PropTypes.func,
+  eventBus: PropTypes.isRequired


is this valid? what's the shape of event bus?

No, this should be an object or an object with { dispatch, subscribe } methods but I am just using object for now

* Allow for optional SimpleCellFormatter.value prop (#948) * Improve Cell navigation Performance (#1123) * Move cell selection responsibilty to SelectionMask component * Selection Mask working at 60fps * Tests for InteractionMask navigation * Editor Container in interaction Layer * commit package-lock.json * Editing Working * smooth scrolling with keyboard navigation * Adjust visible start/end calculations after scroll * Vertical cell Scrolling with keyboard working * generic move command * Vertical scrolling navigation with keyboard working * Code cleanup - Remove extra ref - Format JSX * Update react to the stable version Delete/Ignore package-lock.json * Remove deprecated babel plugins * Export individual constants instead of exporting a default object * Remove react dependency from react-data-grid Delete package.json for now * Remove ReactDOM reference and use refs instead * More code cleanup - Fix formatting - Remove extra spread operator * Add innerRef to fix scrolling Remove unused methods * Remove GridWrapper and GridContainer as it breaks instance methods * Remove unused CellExpander component * - Remove singleton store - Add basic implementation for store/reducer/dispatch - Remove CellContainer to prevent rerendering of each cell on selection change * Delete unused files Cleanup event handlers * Move copy paste functionality to the Interaction mask component Check if cell is editable * Remove flow types * Remove flow types * Remove unused variables * Move copy/paste logic to the interaction layer * Horizontal scroll using keyboard Temporarily disable animation * Rename folder to containers * Fix cell navigation mode * Do not prevent event bubling to show context menu * Add pointer events to fix cell actions * Fix Filter Row focus * Dynamically connect cell to support custom renderers * Fox committing changes Disable cell container component as it degraded performance * Migrate cell drag down to Interaction Layer * Remove react-motion * Add CellMask to highlight copied cell * Add drag mask to highlight dragged cells * Cleanup render method * Add eventbus to communicate with InteractionMask component Delete state and containers * Move EventTypes to contants folder * Fix clicking on a cell scrolls the canvas to the top * Fix cell double click to edit * Fix tests InteractionMasks * Fix issue with press enter not committing * Keep InteractionMask focus logic in central place * Fix canvas tests * Fix Canvas tests * Remove redundant Cell tests * Fix Cell tests and remove redundant ones * Fix clicking on a cell scrolls the canvas to the left Remove componentWillMount * Add a new DragEnter method Some code cleanup * Delete redundant displayStart and displayEnd states * Prevent navigation in FF on drag * Fix Grid.js and remove redundant tests * Move tab behaviour from ReactDataGrid to InteractionMask * Clean up initial tests for tab behavior in InteractionMasks * Fix typo and event name * Use CellNavigationMode constant * Ignore invalid drag items * Use a single div instead of multiple divs for drag mask * Fix SelectionMask zIndex for locked columns * Delete OverflowCell and OverflowRow components * Remove unused props and methods * Clean up props * Fix scrolling when scrollToRowIndex is set * Reorder props for clarity * Commented unused props and methods * Fix ESLint errors * Dispatch SCROLL_TO_COLUMN event to scroll Canvas from ReactDataGrid * Trigger onCellSelected, onCellDeSelected events when cell selection is changed Fix openCellEditor instance method * Fix openCellEditor instance method * Specify required PropTypes * Fix closing tags * Enable auto focus on grid * Fix last row check * Implement tab behavior for InteractionMasks * Fix PropTypes warnings Remove unused state * Fix Cell PropTypes * Fix rowData in EditorContainer * Fix null reference error for drag copy * Check onCheckCellIsEditable for editing * Fix some broken unit tests * Removed redundant and fixed remaining tests * Fix context menu * Fix RowsContainer unit tests * Disable duplicate tests * Fix example home page fonts * Install latest node version * Simplify appveyor script after node upgrade * Fix lint errors * Fix failing IE 11 test * IE compatible version of Array.keys * packages/rdg-test-utils/index.js * Delete unused test-utils project * install gulp gloablly in order to post to coveralls * Use node 6 on appveyor * Delete obsolete scroll utils * Add unit tests for CellMask * Add unit tests for CopyMask * Add unit tests for DragMask * Remove fdescribe * Add unit tests for DragHandle * Add unit tests for EventBus * Format code and object initialization cleanup * Add unit test for copy operation * Use conditional rendering instead of returning null * Add unit tests for Drag functionaity * Add unit tests for the SelectionMask component * Add unit tests for CanvasUtils * Add unit tests for context menu * Add test to check if Canvas renders the InteractionMasks component * Delete commented code * Handle scrolling when selected cell hits the top/bottom boundary when the cellNavigationMode is changeRow * Fix tab behavior * Fix cell selection mask for grouped rows * Add back IE support * Add polyfills for Set and Map to make the examples work in IE10 * Remove unused variable * Fix zIndex for filters * Fix scrolling when editor is closed * Deleted commented code * Fix eslint errors * Delete obsolete tests Remove commented code * Fix Typo * Move the drag enter logic to Row component * Uncomment code * Fix event bus prop type * Define ref callback as a bound method on the class * Remove private getSelectedValue method * Fix Cell component export and columns count (#1227) * Fix cell export * Fix columns count * update file names (#1245) * Rectangular selection range support (on top of 'next') (#1210) * Allow selection of ranges (with mouse & keyboard) Parent components of ReactDataGrid can subscribe to (start, update, end) events about selection ranges. An example page demonstrates this. * Minor InteractionMasks refactoring based on comments on PR #1210 * Update ReactDataGrid API to group cell range event handlers under cellRangeSelection prop * Refactor single and multi-cell range selection masks into components * Revert "Refactor single and multi-cell range selection masks into components" This reverts commit 8ea34d7. * Scrolling improvements (#1254) * Effiecient windowing example * Remove use of refs to setScrollLeft. Renamed row/col visible boundary property names * Calculate overscan columns based on scroll direction * Render overscan columns and rows based on scroll direction * Set height of grid * Improved overscan row/col calcualtion * Send isScrolling prop to formatters to improve perf of complex formatters * Pass isScrolling prop to HeaderCell and Formatter * Pass isScrolling prop to HeaderCell and Formatter 2 * Set pointer-events to none when scrolling * reset setScrollLeft functionality to enable frozen columns * Ensure set scroll left on frozen columns when scrolling in vertical direction * Fix context menu * zindex styles for locked cells * Render overscan columns when row scrolling has complete * Show correct SelectionMask position for locked cells * is scrolling example * Do not deep compare dependentValues check * Ensure rowOverscanEndIdx within bounds * Ensure frozen cells are mounted in correct fixed position if canvas has been scrolled * Move InteractionMasks styles to a separate file * Cherry pick set cell ref commit * Base SelectionMask top and left off rendered cells to handle dynamic row heights * Take scroll left into account when open editor container * Catchup with next branch * Re-add lost canvas.js file * Fix lint errors * Fix some build errors * Remove references to deleted RowsContainer file * Fix typo * Fix InteractionMaks test * Fix some tests * Fix failing tests * Handle dynamic row heights in SelectionMask * Remove unnecessary packages * Viewport scroll calculations * Address pull request comments * use let instead of const in checkScroll function * Remove unneeded id from Row component * Chang logic around getRenderedColumns * Address more PR comments * Tests for Viewport Utils * Fix lint errors and remove console rule from lintrc * Tests for getScrollDirection * Tests for getRowOverscanStartIdx * getRowOverscanEndIdx tests * Tests for getColOverscanEndIdx * Tidy up syntax in example31-isScrolling * Ensure locked columns are shifted to start of column metrics array * Test for rendering the correct cells * Fix lint errors * Rename all fixed/locked/frozen columns to frozen * Fix tests after npm update * Fix IE test * Fixed new warnings in tests * Fix failing test * Update version to 5.0.0-alpha1 * Ensure render all visible columns if scrolled to the right and using frozen columns * Version bump * Change test expectation * Fix test * Ensure last column is rendered when grid scrolled and using frozen columns * Minor version bump * Handle viewport scroll state when no columns present * 5.0.0-alpha8 * Fallback for getSelectedRowTop if no access to row ref * Fix some bugs around cell selection for RowGroups * Fix failing test * Pass row prop to formatters * Fix failing test and version bump * Fix lint error * Re-add deprecated onRowUpdatedMethod - to be removed in v6 * Address some more PR comments * Air BNB browser shims for Enzyme testing * Remove unused functions * prop deprecation warnings * onDragHandleDoubleClick deprecation warning * Publish next branch on commit to next * Get selected row columns if applicable * Fix failing tests * Update npmrc token * Fix some zIndex issues around frozen cells and editors * Remove ReactDataGrid dependency from addons bundle (#1272) * Fix some zIndex issues around frozen cells and editors * Remove rdg dependency from addons package * Fix object assign * Adress PR comments * Fix tests * Fix lint error * add ref to draggable header cell * Adress PR comments * Pass renderBase row and correct ref to custom row renderer * Change editor container to be fixed position * Add missing rdg-editor-container class on EditorContainer (#1275) * Ensure row refs reference an RDG Row component * Show all columns unless scrolling vertically * isFrozen column check * Render edtior container without taking scrollLeft into account if column is frozen * Set CellMask position to be fixed when scrolling horizontally and frozen column is selected * Unit tests and refactoring (#1279) Added unit tests for editor position relating to frozen columns. And refactored spec file, updating legacy code * Upgrade to RDG 5

malonecj added 2 commits February 12, 2018 11:47

Move cell selection responsibilty to SelectionMask component

1458888

Selection Mask working at 60fps

a59e730

This was referenced Feb 12, 2018

Don't autofocus if the entire react-data-grid is offscreen #704

Closed

Re-added selected and active-drag-cell styles #591

Closed

malonecj added 10 commits February 16, 2018 15:32

Tests for InteractionMask navigation

0a426b1

Editor Container in interaction Layer

0cf47d9

Resolve conficts after React 16 catchup

c8730c5

commit package-lock.json

1984d07

Editing Working

e7ae63c

smooth scrolling with keyboard navigation

a19d061

Adjust visible start/end calculations after scroll

13297e1

Vertical cell Scrolling with keyboard working

cbe087b

generic move command

f68bd47

Vertical scrolling navigation with keyboard working

42a1b35

This was referenced Mar 13, 2018

Row content disappears when trying to make table height dynamic #1135

Closed

Values not showing up for right-hand column cells when resize the window #1087

Closed

amanmahajan7 added the WIP label Mar 22, 2018

amanmahajan7 added 13 commits March 22, 2018 16:04

Code cleanup

0a30945

- Remove extra ref - Format JSX

Update react to the stable version

720e071

Delete/Ignore package-lock.json

Remove deprecated babel plugins

98536bd

Export individual constants instead of exporting a default object

0992b49

Remove react dependency from react-data-grid

c6783a9

Delete package.json for now

Remove ReactDOM reference and use refs instead

2b9b22d

More code cleanup

35327e7

- Fix formatting - Remove extra spread operator

Add innerRef to fix scrolling

e350a41

Remove unused methods

Remove GridWrapper and GridContainer as it breaks instance methods

69f719e

Remove unused CellExpander component

7ac93b2

- Remove singleton store

05b62e0

- Add basic implementation for store/reducer/dispatch - Remove CellContainer to prevent rerendering of each cell on selection change

Delete unused files

89337a0

Cleanup event handlers

Move copy paste functionality to the Interaction mask component

c8ed8cb

Check if cell is editable

malonecj and others added 12 commits April 27, 2018 10:31

Fix cell selection mask for grouped rows

8a7d842

Add back IE support

5830a70

Add polyfills for Set and Map to make the examples work in IE10

06c254b

Remove unused variable

418b4dc

Fix zIndex for filters

997d958

Fix scrolling when editor is closed

1834237

Deleted commented code

d135a43

Fix eslint errors

5a13dba

Delete obsolete tests

525fc81

Remove commented code

Fix Typo

ad16034

Move the drag enter logic to Row component

8d7ed63

Uncomment code

2f24e3c

amanmahajan7 reviewed May 1, 2018

View reviewed changes

amanmahajan7 added Performance and removed WIP labels May 1, 2018

richardware requested review from joaodvargas and diogofcunha May 1, 2018 16:18

Resolve conflicts after next catchup

18fd79c

diogofcunha approved these changes May 21, 2018

View reviewed changes

amanmahajan7 added 3 commits May 21, 2018 09:33

Fix event bus prop type

24ee795

Define ref callback as a bound method on the class

ea7b1f3

Remove private getSelectedValue method

6dd8ace

amanmahajan7 merged commit 1c273ed into next May 24, 2018

amanmahajan7 deleted the cm-cell-perf-mask branch May 24, 2018 15:11

rowanhill mentioned this pull request May 26, 2018

Rectangular selection range support (on top of 'next') #1210

Merged

amanmahajan7 mentioned this pull request Jun 6, 2018

Selected cell outline not fully visible in IE11 #1216

Closed

11 tasks

malonecj mentioned this pull request Aug 10, 2018

Scrolling improvements #1254

Merged

10 tasks

amanmahajan7 mentioned this pull request Oct 10, 2019

Delete Grid component #1785

Merged

nstepien mentioned this pull request Jan 16, 2020

rm onRowDoubleClick prop #1900

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Cell navigation Performance #1123

Improve Cell navigation Performance #1123

malonecj commented Feb 12, 2018 •

edited by amanmahajan7

Loading

amanmahajan7 May 1, 2018

amanmahajan7 May 1, 2018

amanmahajan7 May 1, 2018

amanmahajan7 May 1, 2018

amanmahajan7 May 1, 2018

amanmahajan7 May 1, 2018

amanmahajan7 May 1, 2018 •

edited

Loading

diogofcunha left a comment

diogofcunha May 21, 2018

amanmahajan7 May 21, 2018

Improve Cell navigation Performance #1123

Improve Cell navigation Performance #1123

Conversation

malonecj commented Feb 12, 2018 • edited by amanmahajan7 Loading

Description

Results

Tasks

Bugs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amanmahajan7 May 1, 2018 • edited Loading

Choose a reason for hiding this comment

diogofcunha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malonecj commented Feb 12, 2018 •

edited by amanmahajan7

Loading

amanmahajan7 May 1, 2018 •

edited

Loading