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

[Table] Add menu to change cell content in Table playground #1229

Merged
merged 8 commits into from
Jun 14, 2017
Merged
Changes from 6 commits
Commits
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
143 changes: 121 additions & 22 deletions packages/table/preview/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,17 @@ enum FocusStyle {
TAB_OR_CLICK,
};

enum CellContent {
EMPTY,
CELL_NAMES,
LONG_TEXT,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

logically, i'd define this immediately before CELL_CONTENTS instead of inserting this MutableTableState in the way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

k


type IMutableStateUpdateCallback =
(stateKey: keyof IMutableTableState) => ((event: React.FormEvent<HTMLElement>) => void);

interface IMutableTableState {
cellContent?: CellContent;
enableCellEditing?: boolean;
enableCellSelection?: boolean;
enableColumnNameEditing?: boolean;
Expand Down Expand Up @@ -94,15 +104,34 @@ const ROW_COUNTS = [
1000000,
];

const CELL_CONTENTS = [
CellContent.EMPTY,
CellContent.CELL_NAMES,
CellContent.LONG_TEXT,
] as CellContent[];
Copy link
Contributor

Choose a reason for hiding this comment

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

is this cast necessary now that it's an enum? if so, def preferable as type declaration instead of cast.

const CELL_CONTENTS: CellContent[] = ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


const COLUMN_COUNT_DEFAULT_INDEX = 2;
const ROW_COUNT_DEFAULT_INDEX = 3;

const ALPHANUMERIC_CHARS = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";

interface ICellContentGeneratorDictionary {
[key: string]: (rowIndex?: number, columnIndex?: number) => string;
Copy link
Contributor

Choose a reason for hiding this comment

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

key: CellContent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to require [key: number | string] :/

Copy link
Contributor

Choose a reason for hiding this comment

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

how about Record<CellContent, (signature) => string>?

Record is TS's shorthand for this, a la Pick or Partial.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't work:

[ts] Type 'CellContent' does not satisfy the constraint 'string'.
enum CellContent

Using enums as keys is a huge discussion: microsoft/TypeScript#2491. I'll just get rid of the type safety; it's only an example page.

}

class MutableTable extends React.Component<{}, IMutableTableState> {
private store = new SparseGridMutableStore<string>();

private contentGenerators = {
[CellContent.CELL_NAMES]: (row: number, col: number) => Utils.toBase26Alpha(col) + (row + 1),
[CellContent.EMPTY]: () => "",
[CellContent.LONG_TEXT]: () => this.generateRandomAlphanumericString(),
} as ICellContentGeneratorDictionary;

public constructor(props: any, context?: any) {
super(props, context);
this.state = {
cellContent: CellContent.CELL_NAMES,
enableCellEditing: true,
enableCellSelection: true,
enableColumnNameEditing: true,
Expand Down Expand Up @@ -154,7 +183,6 @@ class MutableTable extends React.Component<{}, IMutableTableState> {
selectionModes={this.getEnabledSelectionModes()}
isRowHeaderShown={this.state.showRowHeaders}
styledRegionGroups={this.getStyledRegionGroups()}

onSelection={this.onSelection}
onColumnsReordered={this.onColumnsReordered}
onColumnWidthChanged={this.onColumnWidthChanged}
Expand All @@ -170,10 +198,18 @@ class MutableTable extends React.Component<{}, IMutableTableState> {
);
}

public componentWillMount() {
this.syncCellContent();
}

public componentDidMount() {
this.syncFocusStyle();
}

public componentWillUpdate(_nextProps: {}, nextState: IMutableTableState) {
this.syncCellContent(nextState);
}

public componentDidUpdate() {
this.syncFocusStyle();
}
Expand Down Expand Up @@ -238,17 +274,6 @@ class MutableTable extends React.Component<{}, IMutableTableState> {
}}
text="Remove column"
/>
<MenuItem
iconName="new-text-box"
onClick={() => {
Utils.times(this.state.numRows, (i) => {
const str = Math.random().toString(36).substring(7);
this.store.set(i, columnIndex, str);
});
this.forceUpdate();
}}
text="Fill with random text"
/>
</Menu>;

return this.state.showColumnMenus ? menu : undefined;
Expand Down Expand Up @@ -315,6 +340,13 @@ class MutableTable extends React.Component<{}, IMutableTableState> {
}

private renderSidebar() {
const cellContentMenu = this.renderSelectMenu(
"Cell content",
"cellContent",
CELL_CONTENTS,
this.toCellContentLabel,
this.handleStringStateChange,
);
return (
<div className="sidebar pt-elevation-0">
<h4>Table</h4>
Expand Down Expand Up @@ -353,6 +385,7 @@ class MutableTable extends React.Component<{}, IMutableTableState> {

<h4>Cells</h4>
<h6>Display</h6>
{cellContentMenu}
{this.renderSwitch("Loading state", "showCellsLoading")}
{this.renderSwitch("Custom regions", "showCustomRegions")}
<h6>Interactions</h6>
Expand All @@ -372,7 +405,7 @@ class MutableTable extends React.Component<{}, IMutableTableState> {
checked={this.state[stateKey] as boolean}
className="pt-align-right"
label={label}
onChange={this.updateBooleanState(stateKey)}
onChange={this.handleBooleanStateChange(stateKey)}
/>
);
}
Expand All @@ -397,30 +430,59 @@ class MutableTable extends React.Component<{}, IMutableTableState> {
}

private renderNumberSelectMenu(label: string, stateKey: keyof IMutableTableState, values: number[]) {
const selectedValue = this.state[stateKey] as number;
return this.renderSelectMenu(
label,
stateKey,
values,
this.toValueLabel,
this.handleNumberStateChange,
);
}

private renderSelectMenu<T>(
label: string,
stateKey: keyof IMutableTableState,
values: T[],
generateValueLabel: (value: any) => string,
handleChange: IMutableStateUpdateCallback,
) {
// need to explicitly cast generic type T to string
const selectedValue = this.state[stateKey].toString();
const options = values.map((value) => {
return (
<option key={value} value={value}>
{value}
<option key={value.toString()} value={value.toString()}>
{generateValueLabel(value)}
</option>
);
});
return (
<label className="pt-label pt-inline tbl-select-label">
{label}
<div className="pt-select">
<select onChange={this.updateNumberState(stateKey)} value={selectedValue}>
<select onChange={handleChange(stateKey)} value={selectedValue}>
{options}
</select>
</div>
</label>
);
}
// Select menu - label generators
// ==============================

private toCellContentLabel(cellContent: CellContent) {
if (cellContent === CellContent.CELL_NAMES) { return "Cell names"; }
if (cellContent === CellContent.EMPTY) { return "Empty"; }
if (cellContent === CellContent.LONG_TEXT) { return "Long text"; }
return "";
Copy link
Contributor

Choose a reason for hiding this comment

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

switch? 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, done

}

private toValueLabel(value: any) {
return value.toString();
}

// Callbacks
// =========

// allow console.log for these callbacks so devs can see exactly when they fire
private onSelection = (selectedRegions: IRegion[]) => {
this.maybeLogCallback(`[onSelection] selectedRegions =`, ...selectedRegions);
}
Expand Down Expand Up @@ -451,6 +513,7 @@ class MutableTable extends React.Component<{}, IMutableTableState> {

private maybeLogCallback = (message?: any, ...optionalParams: any[]) => {
if (this.state.showCallbackLogs) {
// allow console.log for these callbacks so devs can see exactly when they fire
// tslint:disable-next-line no-console
console.log(message, ...optionalParams);
}
Expand All @@ -468,6 +531,28 @@ class MutableTable extends React.Component<{}, IMutableTableState> {
// State updates
// =============

// designed to be called from componentWillMount and componentWillUpdate, hence it expects nextProps
private syncCellContent = (nextState?: IMutableTableState) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

(nextState = this.state) => { ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would cause the equality checks below to return on componentWillMount (because the default value would cause isNextStateDefined to never be false); I do want to go through all the content generation at that time, so I'll presumably need to know if the real nextState is defined or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

you could pull that isNextStateDefined check up a level to componentWillUpdate itself, cuz it only applies to that one usage of this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, done

const isNextStateDefined = nextState != null;

if (isNextStateDefined
&& nextState.cellContent === this.state.cellContent
&& nextState.numRows === this.state.numRows
&& nextState.numCols === this.state.numCols
Copy link
Contributor

Choose a reason for hiding this comment

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

inconsistent && alignment here. put all at the beginning of lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops

) {
return;
}

const { cellContent, numRows, numCols } = isNextStateDefined ? nextState : this.state;
const generator = this.contentGenerators[cellContent];

Utils.times(numRows, (rowIndex) => {
Utils.times(numCols, (columnIndex) => {
this.store.set(rowIndex, columnIndex, generator(rowIndex, columnIndex));
});
});
}

private syncFocusStyle() {
const { selectedFocusStyle } = this.state;
const isFocusStyleManagerActive = FocusStyleManager.isActive();
Expand All @@ -479,18 +564,24 @@ class MutableTable extends React.Component<{}, IMutableTableState> {
}
}

private updateBooleanState(stateKey: keyof IMutableTableState) {
private handleBooleanStateChange = (stateKey: keyof IMutableTableState) => {
return handleBooleanChange((value: boolean) => {
this.setState({ [stateKey]: value });
});
}

private updateNumberState(stateKey: keyof IMutableTableState) {
private handleNumberStateChange = (stateKey: keyof IMutableTableState) => {
return handleNumberChange((value: number) => {
this.setState({ [stateKey]: value });
});
}

private handleStringStateChange = (stateKey: keyof IMutableTableState) => {
return handleStringChange((value: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

def don't need the typing on value param in all three cases here, handle*Change does that for you.

then you could make these bodies into one-liners:

private handleNumberStateChange = (stateKey: keyof IMutableTableState) => {
    return handleNumberChange((value) => this.setState({ [stateKey]: value }));
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

this.setState({ [stateKey]: value });
});
}

private updateFocusStyleState = () => {
return handleStringChange((value: string) => {
const selectedFocusStyle = value === "tab" ? FocusStyle.TAB : FocusStyle.TAB_OR_CLICK;
Expand Down Expand Up @@ -560,6 +651,14 @@ class MutableTable extends React.Component<{}, IMutableTableState> {
},
] as IStyledRegionGroup[];
}

private generateRandomAlphanumericString(minLength = 5, maxLength = 40) {
const randomLength = Math.floor(minLength + (Math.random() * (maxLength - minLength)));
return Utils.times(randomLength, () => {
const randomIndex = Math.floor(Math.random() * maxLength);
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seem safe... did you mean ALPHANUMERIC_CHARS.length?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did.

return ALPHANUMERIC_CHARS[randomIndex];
}).join("");
}
}

ReactDOM.render(
Expand All @@ -574,12 +673,12 @@ function handleBooleanChange(handler: (checked: boolean) => void) {
return (event: React.FormEvent<HTMLElement>) => handler((event.target as HTMLInputElement).checked);
}

// /** Event handler that exposes the target element's value as a string. */
/** Event handler that exposes the target element's value as a string. */
function handleStringChange(handler: (value: string) => void) {
return (event: React.FormEvent<HTMLElement>) => handler((event.target as HTMLInputElement).value);
}

// /** Event handler that exposes the target element's value as a number. */
/** Event handler that exposes the target element's value as a number. */
function handleNumberChange(handler: (value: number) => void) {
return handleStringChange((value) => handler(+value));
}