make scrollTo{Column,Row} take precedence (bvaughn#1130)

* make scrollTo{Column,Row} take precedence * typo
IncepAndrasBalogh · Jun 13, 2018 · c1d7377 · c1d7377
1 parent dcef5bc
commit c1d7377
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 31 deletions.
diff --git a/docs/Grid.md b/docs/Grid.md
@@ -36,8 +36,8 @@ A windowed grid of elements. `Grid` only renders cells necessary to fill itself
 | scrollingResetTimeInterval | Number |  | Wait this amount of time after the last scroll event before resetting Grid `pointer-events`; defaults to 150ms. |
 | scrollLeft | Number |  | Horizontal offset |
 | scrollToAlignment | String |  | Controls the alignment of scrolled-to-cells. The default ("_auto_") scrolls the least amount possible to ensure that the specified cell is fully visible. Use "_start_" to always align cells to the top/left of the `Grid` and "_end_" to align them bottom/right. Use "_center_" to align specified cell in the middle of container. |
-| scrollToColumn | Number |  | Column index to ensure visible (by forcefully scrolling if necessary) |
-| scrollToRow | Number |  | Row index to ensure visible (by forcefully scrolling if necessary) |
+| scrollToColumn | Number |  | Column index to ensure visible (by forcefully scrolling if necessary). Takes precedence over `scrollLeft`. |
+| scrollToRow | Number |  | Row index to ensure visible (by forcefully scrolling if necessary). Takes precedence over `scrollTop`. |
 | scrollTop | Number |  | Vertical offset |
 | style | Object |  | Optional custom inline style to attach to root `Grid` element. |
 | tabIndex | Number |  | Optional override of tab index default; defaults to `0`. |

diff --git a/source/Grid/Grid.js b/source/Grid/Grid.js
@@ -814,37 +814,22 @@ class Grid extends React.PureComponent<Props, State> {
     ) {
       newState.scrollLeft = 0;
       newState.scrollTop = 0;
+
+      // only use scroll{Left,Top} from props if scrollTo{Column,Row} isn't specified
+      // scrollTo{Column,Row} should override scroll{Left,Top}
     } else if (
-      nextProps.scrollLeft !== prevState.scrollLeft ||
-      nextProps.scrollTop !== prevState.scrollTop
+      (nextProps.scrollLeft !== prevState.scrollLeft &&
+        nextProps.scrollToColumn < 0) ||
+      (nextProps.scrollTop !== prevState.scrollTop && nextProps.scrollToRow < 0)
     ) {
-      // this handles the weird edge case where setting scrollToColumn in
-      // multigrid was causing multiple getDerivedStateFromProps calls. Overriding
-      // the state set in the first time.
-      // We should warn since if scrollTo{column,row} and scroll{left,top} is specified,
-      // scrollTo{column,row} should override.
-      if (
-        nextProps.scrollToColumn > 0 &&
-        prevState.scrollLeft &&
-        nextProps.scrollLeft === 0
-      ) {
-        //NOOP
-      } else if (
-        nextProps.scrollToRow > 0 &&
-        prevState.scrollTop &&
-        nextProps.scrollTop === 0
-      ) {
-        //NOOP
-      } else {
-        Object.assign(
-          newState,
-          Grid._getScrollToPositionStateUpdate({
-            prevState,
-            scrollLeft: nextProps.scrollLeft,
-            scrollTop: nextProps.scrollTop,
-          }),
-        );
-      }
+      Object.assign(
+        newState,
+        Grid._getScrollToPositionStateUpdate({
+          prevState,
+          scrollLeft: nextProps.scrollLeft,
+          scrollTop: nextProps.scrollTop,
+        }),
+      );
     }
 
     let {instanceProps} = prevState;