-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[Table] scrollToRegion instance method #1496
Changes from 29 commits
f661262
bd91c72
c4d9f6b
9e694ab
70a8bb5
bd1c170
f65c8f4
0ad4d8a
33bb63b
b4cf85e
cc7b908
fcb8aed
2c29b21
a29f832
a87242d
5cbf3b6
ca9be6b
f5080e4
28fcb52
9d1d9f1
5281104
839c027
89b7b05
da186b5
dfb5a9c
26b4fc8
70ca679
f21b683
ec3e915
80dfb99
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,8 +12,10 @@ import * as React from "react"; | |
import * as ReactDOM from "react-dom"; | ||
|
||
import { | ||
Button, | ||
Classes, | ||
FocusStyleManager, | ||
Intent, | ||
Menu, | ||
MenuDivider, | ||
MenuItem, | ||
|
@@ -73,6 +75,9 @@ interface IMutableTableState { | |
numFrozenCols?: number; | ||
numFrozenRows?: number; | ||
numRows?: number; | ||
scrollToColumnIndex?: number; | ||
scrollToRegionType?: RegionCardinality; | ||
scrollToRowIndex?: number; | ||
selectedFocusStyle?: FocusStyle; | ||
showCallbackLogs?: boolean; | ||
showCellsLoading?: boolean; | ||
|
@@ -110,6 +115,13 @@ const ROW_COUNTS = [ | |
const FROZEN_COLUMN_COUNTS = [0, 1, 2, 5, 20, 100, 1000]; | ||
const FROZEN_ROW_COUNTS = [0, 1, 2, 5, 20, 100, 1000]; | ||
|
||
const REGION_CARDINALITIES = [ | ||
RegionCardinality.CELLS, | ||
RegionCardinality.FULL_ROWS, | ||
RegionCardinality.FULL_COLUMNS, | ||
RegionCardinality.FULL_TABLE, | ||
]; | ||
|
||
enum CellContent { | ||
EMPTY, | ||
CELL_NAMES, | ||
|
@@ -153,6 +165,12 @@ const CELL_CONTENT_GENERATORS = { | |
class MutableTable extends React.Component<{}, IMutableTableState> { | ||
private store = new DenseGridMutableStore<string>(); | ||
|
||
private tableInstance: Table; | ||
|
||
private refHandlers = { | ||
table: (ref: Table) => this.tableInstance = ref, | ||
}; | ||
|
||
public constructor(props: any, context?: any) { | ||
super(props, context); | ||
|
||
|
@@ -176,6 +194,9 @@ class MutableTable extends React.Component<{}, IMutableTableState> { | |
numFrozenCols: FROZEN_COLUMN_COUNTS[FROZEN_COLUMN_COUNT_DEFAULT_INDEX], | ||
numFrozenRows: FROZEN_ROW_COUNTS[FROZEN_ROW_COUNT_DEFAULT_INDEX], | ||
numRows: ROW_COUNTS[ROW_COUNT_DEFAULT_INDEX], | ||
scrollToColumnIndex: 0, | ||
scrollToRegionType: RegionCardinality.CELLS, | ||
scrollToRowIndex: 0, | ||
selectedFocusStyle: FocusStyle.TAB, | ||
showCallbackLogs: false, | ||
showCellsLoading: false, | ||
|
@@ -210,6 +231,7 @@ class MutableTable extends React.Component<{}, IMutableTableState> { | |
isRowResizable={this.state.enableRowResizing} | ||
loadingOptions={this.getEnabledLoadingOptions()} | ||
numRows={this.state.numRows} | ||
ref={this.refHandlers.table} | ||
renderBodyContextMenu={this.renderBodyContextMenu} | ||
renderRowHeader={this.renderRowHeader} | ||
selectionModes={this.getEnabledSelectionModes()} | ||
|
@@ -432,6 +454,8 @@ class MutableTable extends React.Component<{}, IMutableTableState> { | |
{this.renderSwitch("Callback logs", "showCallbackLogs")} | ||
{this.renderSwitch("Full-table selection", "enableFullTableSelection")} | ||
{this.renderSwitch("Multi-selection", "enableMultiSelection")} | ||
<h6>Scroll to</h6> | ||
{this.renderScrollToSection()} | ||
|
||
<h4>Columns</h4> | ||
<h6>Display</h6> | ||
|
@@ -479,6 +503,66 @@ class MutableTable extends React.Component<{}, IMutableTableState> { | |
); | ||
} | ||
|
||
private renderScrollToSection() { | ||
const { scrollToRegionType } = this.state; | ||
|
||
const scrollToRegionTypeSelectMenu = this.renderSelectMenu( | ||
"Region type", | ||
"scrollToRegionType", | ||
REGION_CARDINALITIES, | ||
this.getRegionCardinalityLabel, | ||
this.handleRegionCardinalityChange, | ||
); | ||
const scrollToRowSelectMenu = this.renderSelectMenu( | ||
"Row", | ||
"scrollToRowIndex", | ||
Utils.times(this.state.numRows, (rowIndex) => rowIndex), | ||
(rowIndex) => `${rowIndex + 1}`, | ||
this.handleNumberStateChange, | ||
); | ||
const scrollToColumnSelectMenu = this.renderSelectMenu( | ||
"Column", | ||
"scrollToColumnIndex", | ||
Utils.times(this.state.numCols, (columnIndex) => columnIndex), | ||
(columnIndex) => this.store.getColumnName(columnIndex), | ||
this.handleNumberStateChange, | ||
); | ||
|
||
const ROW_MENU_CARDINALITIES = [RegionCardinality.CELLS, RegionCardinality.FULL_ROWS]; | ||
const COLUMN_MENU_CARDINALITIES = [RegionCardinality.CELLS, RegionCardinality.FULL_COLUMNS]; | ||
|
||
const shouldShowRowSelectMenu = contains(ROW_MENU_CARDINALITIES, scrollToRegionType); | ||
const shouldShowColumnSelectMenu = contains(COLUMN_MENU_CARDINALITIES, scrollToRegionType); | ||
|
||
return ( | ||
<div> | ||
{scrollToRegionTypeSelectMenu} | ||
<div className="sidebar-indented-group"> | ||
{shouldShowRowSelectMenu ? scrollToRowSelectMenu : undefined} | ||
{shouldShowColumnSelectMenu ? scrollToColumnSelectMenu : undefined} | ||
</div> | ||
<Button intent={Intent.PRIMARY} className={Classes.FILL} onClick={this.handleScrollToButtonClick}> | ||
Scroll | ||
</Button> | ||
</div> | ||
); | ||
} | ||
|
||
private getRegionCardinalityLabel(cardinality: RegionCardinality) { | ||
switch (cardinality) { | ||
case RegionCardinality.CELLS: | ||
return "Cell"; | ||
case RegionCardinality.FULL_ROWS: | ||
return "Row"; | ||
case RegionCardinality.FULL_COLUMNS: | ||
return "Column"; | ||
case RegionCardinality.FULL_TABLE: | ||
return "Full table"; | ||
default: | ||
return ""; | ||
} | ||
} | ||
|
||
private renderSwitch( | ||
label: string, | ||
stateKey: keyof IMutableTableState, | ||
|
@@ -683,6 +767,30 @@ class MutableTable extends React.Component<{}, IMutableTableState> { | |
this.store.setColumnName(columnIndex, value); | ||
} | ||
|
||
private handleScrollToButtonClick = () => { | ||
const { scrollToRowIndex, scrollToColumnIndex, scrollToRegionType } = this.state; | ||
|
||
let region: IRegion; | ||
switch (scrollToRegionType) { | ||
case RegionCardinality.CELLS: | ||
region = Regions.cell(scrollToRowIndex, scrollToColumnIndex); | ||
break; | ||
case RegionCardinality.FULL_ROWS: | ||
region = Regions.row(scrollToRowIndex); | ||
break; | ||
case RegionCardinality.FULL_COLUMNS: | ||
region = Regions.column(scrollToColumnIndex); | ||
break; | ||
case RegionCardinality.FULL_TABLE: | ||
region = Regions.table(); | ||
break; | ||
default: | ||
return; | ||
} | ||
|
||
this.tableInstance.scrollToRegion(region); | ||
} | ||
|
||
// State updates | ||
// ============= | ||
|
||
|
@@ -724,6 +832,10 @@ class MutableTable extends React.Component<{}, IMutableTableState> { | |
return handleNumberChange((value) => this.setState({ [stateKey]: value })); | ||
} | ||
|
||
private handleRegionCardinalityChange = (stateKey: keyof IMutableTableState) => { | ||
return handleNumberChange((value) => this.setState({ [stateKey]: value })); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. man this pattern is so common, i wish react had a shorthand for this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e.g. 4 lines above |
||
} | ||
|
||
private updateFocusStyleState = () => { | ||
return handleStringChange((value: string) => { | ||
const selectedFocusStyle = value === "tab" ? FocusStyle.TAB : FocusStyle.TAB_OR_CLICK; | ||
|
@@ -821,3 +933,7 @@ function getRandomInteger(min: number, max: number) { | |
// min and max are inclusive | ||
return Math.floor(min + (Math.random() * (max - min + 1))); | ||
} | ||
|
||
function contains(arr: any[], value: any) { | ||
return arr.indexOf(value) >= 0; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,4 +10,7 @@ export { Grid } from "./grid"; | |
export { Rect, AnyRect } from "./rect"; | ||
export { RoundSize } from "./roundSize"; | ||
export { Utils } from "./utils"; | ||
// NOTE: Errors is not exported in public API | ||
|
||
// NOTE: The following are not exported in the public API: | ||
// - Errors | ||
// - internal/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thoughts on this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 yes nice |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Really want to get into the practice of taking utility code out of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's cool, if you add more do you think it's useful to keep track of an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd be into that. 👍 Another immediate idea is |
||
* Copyright 2017 Palantir Technologies, Inc. All rights reserved. | ||
* Licensed under the BSD-3 License as modified (the “License”); you may obtain a copy | ||
* of the license at https://github.com/palantir/blueprint/blob/master/LICENSE | ||
* and https://github.com/palantir/blueprint/blob/master/PATENTS | ||
*/ | ||
|
||
import { IRegion, RegionCardinality, Regions } from "../../regions"; | ||
|
||
export function getScrollPositionForRegion( | ||
region: IRegion, | ||
currScrollLeft: number, | ||
currScrollTop: number, | ||
getLeftOffset: (columnIndex: number) => number, | ||
getTopOffset: (rowIndex: number) => number, | ||
numFrozenRows: number = 0, | ||
numFrozenColumns: number = 0, | ||
) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A lot of params, but this way I don't have to pass a bloated There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems legit There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeeesh. how many args would passing those props around save? this is an internal method and those args are passed by reference, so no perf penalty. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would save two args ( Also, we don't really need to pass in the current scroll offsets. We could simply have this function return region: IRegion;
getLeftOffset: (columnIndex: number) => number;
getTopOffset: (rowIndex: number) => number;
numFrozenRows: number = 0;
numFrozenColumns: number = 0; Thoughts? @themadcreator @giladgray EDIT: Actually, I guess this isn't quite true given the need for clamping of values within the frozen area. Ignore the above. |
||
const cardinality = Regions.getRegionCardinality(region); | ||
|
||
let scrollTop = currScrollTop; | ||
let scrollLeft = currScrollLeft; | ||
|
||
// if these were max-frozen-index values, we would have added 1 before passing to the get*Offset | ||
// functions, but the counts are already 1-indexed, so we can just pass those. | ||
const frozenColumnsCumulativeWidth = getLeftOffset(numFrozenColumns); | ||
const frozenRowsCumulativeHeight = getTopOffset(numFrozenRows); | ||
|
||
switch (cardinality) { | ||
case RegionCardinality.CELLS: { | ||
// scroll to the top-left corner of the block of cells | ||
const topOffset = getTopOffset(region.rows[0]); | ||
const leftOffset = getLeftOffset(region.cols[0]); | ||
scrollTop = getClampedScrollPosition(topOffset, frozenRowsCumulativeHeight); | ||
scrollLeft = getClampedScrollPosition(leftOffset, frozenColumnsCumulativeWidth); | ||
break; | ||
} | ||
case RegionCardinality.FULL_ROWS: { | ||
// scroll to the top of the row block | ||
const topOffset = getTopOffset(region.rows[0]); | ||
scrollTop = getClampedScrollPosition(topOffset, frozenRowsCumulativeHeight); | ||
break; | ||
} | ||
case RegionCardinality.FULL_COLUMNS: { | ||
// scroll to the left side of the column block | ||
const leftOffset = getLeftOffset(region.cols[0]); | ||
scrollLeft = getClampedScrollPosition(leftOffset, frozenColumnsCumulativeWidth); | ||
break; | ||
} | ||
default: { | ||
// if it's a FULL_TABLE region, scroll back to the top-left cell of the table | ||
scrollTop = 0; | ||
scrollLeft = 0; | ||
break; | ||
} | ||
} | ||
|
||
return { scrollLeft, scrollTop }; | ||
} | ||
|
||
/** | ||
* Adjust the scroll position to align content just beyond the frozen region, if necessary. | ||
*/ | ||
function getClampedScrollPosition(scrollOffset: number, frozenRegionCumulativeSize: number) { | ||
// if the new scroll offset falls within the frozen region, clamp it to 0 | ||
return Math.max(scrollOffset - frozenRegionCumulativeSize, 0); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -192,6 +192,17 @@ export class TableQuadrantStack extends AbstractComponent<ITableQuadrantStackPro | |
this.throttledHandleWheel = CoreUtils.throttleReactEventCallback(this.handleWheel, { preventDefault: true }); | ||
} | ||
|
||
/** | ||
* Scroll the main quadrant to the specified scroll offset, keeping all other quadrants in sync. | ||
*/ | ||
public scrollToPosition(scrollLeft: number, scrollTop: number) { | ||
const { scrollContainer } = this.quadrantRefs[QuadrantType.MAIN]; | ||
this.wasMainQuadrantScrollChangedFromOtherOnWheelCallback = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. now THAT is a variable name. 💯 |
||
// this will trigger the main quadrant's scroll callback below | ||
scrollContainer.scrollLeft = scrollLeft; | ||
scrollContainer.scrollTop = scrollTop; | ||
} | ||
|
||
public componentDidMount() { | ||
this.emitRefs(); | ||
this.syncQuadrantSizes(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is invoking methods on the table ref the standard way for actions like this? do we have other components/features that do this? is this what users will expect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess the resizeRows does this. still, is this the idiomatic blueprint way to invoke actions on react components?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there are a few instance methods around the codebase, each of which operates on a
ref
. The goal here was to provide a way to briefly control the scroll without forcing the caller to control it forever and always (would be annoying and unnecessary). See: