Skip to content

Commit

Permalink
fix: Grid rendering header incorrectly when hiding all children in a …
Browse files Browse the repository at this point in the history
…group via layout hints (#1139)

Fixes #1097 

Also updated the readme for updating e2e snapshots and removed some env
variables that shouldn't be needed any more when updating/running e2e
tests

---------

Co-authored-by: mikebender <mikebender@deephaven.io>
  • Loading branch information
mattrunyon and mofojed authored Mar 8, 2023
1 parent 46c5e24 commit 2fbccc6
Show file tree
Hide file tree
Showing 13 changed files with 40 additions and 10 deletions.
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,11 @@ npx source-map-explorer 'packages/code-studio/build/static/js/*.js'

## Updating Snapshots

Snapshots are used by end-to-end tests to visually verify the output. Sometimes changes are made requiring snapshots to be updated. Run snapshots locally to update first, by running `npm run e2e:update-snapshots`.
**Note:** You must have [Docker installed](https://docs.docker.com/get-docker/), and `deephaven-core` must already be running on port 10000 on your local machine for all e2e tests.

Once you are satisfied with the snapshots and everything is passing locally, you need to use a docker image to [update snapshots for CI](https://playwright.dev/docs/test-snapshots) (unless you are running the same platform as CI (Ubuntu)). Run `npm run e2e:update-ci-snapshots` to mount the current directory into a docker image and re-run the tests from there. **Note:** You must have [Docker installed](https://docs.docker.com/get-docker/), and `deephaven-core` must already be running on port 10000 on your local machine.
Snapshots are used by end-to-end tests to visually verify the output. Sometimes changes are made requiring snapshots to be updated. Before running snapshots locally, you must be running the development server with `npm start` or have run `npm run build` recently. Run snapshots locally by running `npm run e2e:update-snapshots`.

Once you are satisfied with the snapshots and everything is passing locally, you need to use a docker image to [update snapshots for CI](https://playwright.dev/docs/test-snapshots) (unless you are running the same platform as CI (Ubuntu)). Run `npm run e2e:update-ci-snapshots` which will mount the current directory into a docker image and re-run the tests from there. Test results will be written to your local directories.

## Updating Dependencies

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"e2e:headed": "playwright test --project=chromium --headed --debug",
"e2e:update-snapshots": "playwright test --update-snapshots",
"version-bump": "lerna version --conventional-commits --create-release github",
"e2e:update-ci-snapshots": "docker build --tag web-client-ui-ci-snapshots --file ./tests/update-ci-snapshots/Dockerfile . && docker run --rm --network host --volume $(pwd)/tests:/work/tests/ web-client-ui-ci-snapshots npm run e2e:update-snapshots -- --config=playwright-ci.config.ts"
"e2e:update-ci-snapshots": "docker build --tag web-client-ui-ci-snapshots --file ./tests/update-ci-snapshots/Dockerfile . && docker run --rm --network host --volume $(pwd)/tests:/work/tests/ --volume $(pwd)/playwright-report:/work/playwright-report --volume $(pwd)/test-results:/work/test-results web-client-ui-ci-snapshots npm run e2e:update-snapshots -- --config=playwright-ci.config.ts"
},
"repository": "https://github.com/deephaven/web-client-ui",
"devDependencies": {
Expand Down
7 changes: 2 additions & 5 deletions packages/grid/src/GridRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1592,7 +1592,6 @@ export class GridRenderer {
modelColumns,
allColumnXs,
gridX,
calculatedColumnWidths,
userColumnWidths,
allColumnWidths,
movedColumns,
Expand Down Expand Up @@ -1660,10 +1659,9 @@ export class GridRenderer {
break;
}

// Use this instead of visibleColumnWidths b/c the columns may be off screen
const prevColumnWidth =
userColumnWidths.get(prevModelIndex) ??
calculatedColumnWidths.get(prevModelIndex) ??
allColumnWidths.get(prevColumnIndex) ??
columnWidth;

columnGroupLeft -= prevColumnWidth;
Expand All @@ -1687,10 +1685,9 @@ export class GridRenderer {
break;
}

// Use this instead of visibleColumnWidths b/c the columns may be off screen
const nextColumnWidth =
userColumnWidths.get(nextModelIndex) ??
calculatedColumnWidths.get(nextModelIndex) ??
allColumnWidths.get(nextColumnIndex) ??
columnWidth;

columnGroupRight += nextColumnWidth;
Expand Down
2 changes: 1 addition & 1 deletion playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ const config: PlaywrightTestConfig = {
webServer: {
// Only start the main code-studio server right now
// To test embed-grid and embed-chart, should have an array set for `webServer` and run them all separately as there's a port check
command: 'VITE_PROXY_URL=http://localhost:10000 npm run preview:app',
command: 'npm run preview:app -- -- -- --no-open', // Passing flags through npm is fun
port: 4000,
timeout: 60 * 1000,
reuseExistingServer: !process.env.CI,
Expand Down
31 changes: 31 additions & 0 deletions tests/table.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,37 @@ column_header_group = column_header_group.layout_hints(column_groups=column_grou
await expect(page.locator('.iris-grid-panel .iris-grid')).toHaveScreenshot();
});

test('can open a table with column header groups and hidden columns', async ({
page,
}) => {
await page.goto('');
const consoleInput = page.locator('.console-input');
await consoleInput.click();

const command = `${makeTableCommand('column_header_group')}
column_groups = [{ 'name': 'YandZ', 'children': ['y', 'z'] }, { 'name': 'All', 'children': ['x', 'YandZ'], 'color': 'white' }]
column_header_group = column_header_group.layout_hints(column_groups=column_groups, hide=['y', 'z'])`;

await pasteInMonaco(consoleInput, command);
await page.keyboard.press('Enter');

// Wait for the panel to show
await expect(page.locator('.iris-grid-panel')).toHaveCount(1);

// Wait until it's done loading
await expect(page.locator('.iris-grid-panel .loading-spinner')).toHaveCount(
0
);

// Model is loaded, need to make sure table data is also loaded
await expect(
page.locator('.iris-grid .iris-grid-loading-status')
).toHaveCount(0);

// Now we should be able to check the snapshot
await expect(page.locator('.iris-grid-panel .iris-grid')).toHaveScreenshot();
});

test.describe('tests table operations', () => {
let page: Page;

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion tests/update-ci-snapshots/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,4 @@ RUN SKIP_POSTINSTALL=1 npm ci
COPY . .

# Now build the app. We only need the code-studio built for e2e tests.
RUN VITE_CORE_API_URL=http://host.docker.internal:10000/jsapi npm run build:app
RUN npm run build:app

0 comments on commit 2fbccc6

Please sign in to comment.