Skip to content

Commit

Permalink
fix: keep manually entered value in GoToRow when changing to same col…
Browse files Browse the repository at this point in the history
…umn type (#1743)

- Fixes #1562 
  - Tracks if the value was manually entered and changes appropriately
  - Adds test with a wait to account for the debounce

**Testing Methodology**
- Generated a relevant table with 2 integer and 1 string columns
- GoToRow has a column selector and a field to input the value, the
possible configurations are
  - If the column selector has changed types
  - If the field was changed
- GoToRow should leave the field unmodified if the column selector has
changed types and the field was changed, otherwise keep the original
behaviour
- One special case where this is the case AND GoToRow has been toggled,
going back to the original behaviour
- Helper function `setColumnAndExpectInputValue` to make the changes and
do the test
  • Loading branch information
wusteven815 authored Jan 25, 2024
1 parent 0ab0c93 commit 689a1e2
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 8 deletions.
1 change: 1 addition & 0 deletions packages/iris-grid/src/GotoRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ const GotoRow = forwardRef<GotoRowElement, GotoRowProps>(
}}
value={gotoValueSelectedColumnName}
aria-label="column-name-select"
id="column-name-select"
>
{columns.map(column => (
<option key={column.name} value={column.name}>
Expand Down
40 changes: 32 additions & 8 deletions packages/iris-grid/src/IrisGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ export interface IrisGridState {

gotoValueSelectedColumnName: ColumnName;
gotoValueSelectedFilter: FilterTypeValue;
gotoValueManuallyChanged: boolean;
gotoValue: string;

columnHeaderGroups: readonly ColumnHeaderGroup[];
Expand Down Expand Up @@ -851,6 +852,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
gotoValueSelectedColumnName: model.columns[0]?.name ?? '',
gotoValueSelectedFilter: FilterType.eqIgnoreCase,
gotoValue: '',
gotoValueManuallyChanged: false,
columnHeaderGroups: columnHeaderGroups ?? model.initialColumnHeaderGroups,
};
}
Expand Down Expand Up @@ -2606,6 +2608,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
gotoValueSelectedColumnName: columnName,
gotoRowError: '',
gotoValueError: '',
gotoValueManuallyChanged: false,
});
this.focusRowInGrid(row);
this.gotoRowRef.current?.focus();
Expand All @@ -2623,6 +2626,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
gotoValue: '',
gotoRowError: '',
gotoValueError: '',
gotoValueManuallyChanged: false,
});
return;
}
Expand All @@ -2639,6 +2643,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
gotoValueSelectedColumnName: name,
gotoRowError: '',
gotoValueError: '',
gotoValueManuallyChanged: false,
});
}

Expand Down Expand Up @@ -2787,11 +2792,11 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
}

handleGotoRowOpened(): void {
this.setState({ isGotoShown: true });
this.setState({ isGotoShown: true, gotoValueManuallyChanged: false });
}

handleGotoRowClosed(): void {
this.setState({ isGotoShown: false });
this.setState({ isGotoShown: false, gotoValueManuallyChanged: false });
}

handleAdvancedMenuClosed(columnIndex: number): void {
Expand Down Expand Up @@ -3935,20 +3940,39 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
handleGotoValueSelectedColumnNameChanged(columnName: ColumnName): void {
const { model } = this.props;
const cursorRow = this.grid?.state.cursorRow;
const {
gotoValueSelectedColumnName: prevColumnName,
gotoValueManuallyChanged,
} = this.state;

if (cursorRow != null) {
const index = model.getColumnIndexByName(columnName);
const column = IrisGridUtils.getColumnByName(model.columns, columnName);
const prevColumn = IrisGridUtils.getColumnByName(
model.columns,
prevColumnName
);
if (index == null || column == null) {
return;
}
const value = model.valueForCell(index, cursorRow);
const text = IrisGridUtils.convertValueToText(value, column.type);
this.setState({
gotoValueSelectedColumnName: columnName,
gotoValue: text,
gotoValueError: '',
});

// do NOT update value if user manually changed value AND column type remains the same
if (gotoValueManuallyChanged && column.type === prevColumn?.type) {
this.setState({
gotoValueSelectedColumnName: columnName,
gotoValueError: '',
});
} else {
// do update, and set goToValueManuallyChanged to false because value was automatically changed
this.setState({
gotoValueSelectedColumnName: columnName,
gotoValue: text,
gotoValueError: '',
gotoValueManuallyChanged: false,
});
}
}
this.setState({
gotoValueSelectedColumnName: columnName,
Expand All @@ -3961,7 +3985,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
}

handleGotoValueChanged = (input: string): void => {
this.setState({ gotoValue: input });
this.setState({ gotoValue: input, gotoValueManuallyChanged: true });
this.debouncedSeekRow(input);
};

Expand Down
179 changes: 179 additions & 0 deletions tests/table-gotorow.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import { test, expect, Page } from '@playwright/test';
import { pasteInMonaco } from './utils';

const customTableCommand = `
from deephaven import empty_table
size = 20
ordered_int_and_offset = empty_table(20).update([
"MyString=(\`str\`+i)",
"MyInt1=(i+100)",
"MyInt2=(i+200)",
])`;

async function setColumnAndExpectInputValue({
page,
setInputValueTo,
toggleGoToRow = false,
setColumnNameTo,
expectInputValueToBe,
}: {
page: Page;
setInputValueTo?: string;
toggleGoToRow?: boolean;
setColumnNameTo: string;
expectInputValueToBe: string;
}) {
// get the input value and set it
if (setInputValueTo !== undefined) {
const inputValue = page.locator('input[aria-label="Value Input"]');
await expect(inputValue).toHaveCount(1);
await inputValue.fill(setInputValueTo);
await page.waitForTimeout(300);
}

if (toggleGoToRow) {
await page.keyboard.down('Control');
await page.keyboard.press('g');
await page.keyboard.press('g');
await page.keyboard.up('Control');
}

// change the column select to the target
const columnSelect = page.locator('#column-name-select');
await expect(columnSelect).toHaveCount(1);
await columnSelect.selectOption(setColumnNameTo);

// check the input value
const inputValue = page.locator('input[aria-label="Value Input"]');
await expect(inputValue).toHaveCount(1);
await expect(inputValue).toHaveValue(expectInputValueToBe);
}

async function waitForLoadingDone(page: Page) {
await expect(
page.locator('.iris-grid .iris-grid-loading-status')
).toHaveCount(0);
}

test.describe.configure({ mode: 'serial' });

test.describe('GoToRow change column', () => {
let page: Page;

test.beforeAll(async ({ browser }) => {
page = await browser.newPage();
await page.goto('');
await waitForLoadingDone(page);

// create the table
const consoleInput = page.locator('.console-input');
await pasteInMonaco(consoleInput, customTableCommand);
await page.keyboard.press('Enter');

// wait for panel to show and finish loading
await expect(page.locator('.iris-grid-panel')).toHaveCount(1);
await expect(page.locator('.iris-grid-panel .loading-spinner')).toHaveCount(
0
);

// get the grid
const grid = await page.locator('.iris-grid-panel .iris-grid');
await waitForLoadingDone(page);
const gridLocation = await grid.boundingBox();
expect(gridLocation).not.toBeNull();
if (gridLocation === null) return;
// activate GoToRow
const columnHeaderHeight = 30;
await page.mouse.click(
gridLocation.x + 1,
gridLocation.y + 1 + columnHeaderHeight
);
await page.keyboard.down('Control');
await page.keyboard.press('g');
await page.keyboard.up('Control');

// wait for GoToRow bar to show
await expect(page.locator('.iris-grid-bottom-bar')).toHaveCount(1);
});

test('unmodified set value, different column type > change value', async () => {
await setColumnAndExpectInputValue({
page,
setColumnNameTo: 'MyInt1',
expectInputValueToBe: '100',
});
await setColumnAndExpectInputValue({
page,
setColumnNameTo: 'MyString',
expectInputValueToBe: 'str0',
});
});

test('modified set value, different column type > change value', async () => {
await setColumnAndExpectInputValue({
page,
setInputValueTo: 'str5',
setColumnNameTo: 'MyInt1',
expectInputValueToBe: '105',
});
await setColumnAndExpectInputValue({
page,
setInputValueTo: '110',
setColumnNameTo: 'MyString',
expectInputValueToBe: 'str10',
});
});

test('unmodified set value, same column type > change value', async () => {
// set to int1 first (from string)
await setColumnAndExpectInputValue({
page,
setColumnNameTo: 'MyInt1',
expectInputValueToBe: '110',
});
await setColumnAndExpectInputValue({
page,
setColumnNameTo: 'MyInt2',
expectInputValueToBe: '210',
});
await setColumnAndExpectInputValue({
page,
setColumnNameTo: 'MyInt1',
expectInputValueToBe: '110',
});
});

test('modified set value, same column type > keep value', async () => {
await setColumnAndExpectInputValue({
page,
setInputValueTo: '115',
setColumnNameTo: 'MyInt2',
expectInputValueToBe: '115',
});
await setColumnAndExpectInputValue({
page,
setInputValueTo: '216',
setColumnNameTo: 'MyInt1',
expectInputValueToBe: '216',
});
});

test('modified set value, same column type, toggled GoToRow > change value', async () => {
await setColumnAndExpectInputValue({
page,
setInputValueTo: '110',
toggleGoToRow: true,
setColumnNameTo: 'MyInt2',
expectInputValueToBe: '210',
});
await setColumnAndExpectInputValue({
page,
setInputValueTo: '212',
toggleGoToRow: true,
setColumnNameTo: 'MyInt1',
expectInputValueToBe: '112',
});
});
});

0 comments on commit 689a1e2

Please sign in to comment.