Skip to content

Commit

Permalink
Merge pull request #2353 from finos/fix-filter-validation
Browse files Browse the repository at this point in the history
Fix filter number validation with trailing zeroes
  • Loading branch information
texodus authored Aug 31, 2023
2 parents 01cbbb0 + 56ec825 commit 285a76c
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const FORMAT_CACHE = new FormatterCache();
* @returns
*/
export function format_cell(
parts,
title,
val,
plugins = {},
use_table_schema = false
Expand All @@ -33,8 +33,6 @@ export function format_cell(
return "-";
}

// TODO don't do this on every cell render ...
const title = parts[this._config.split_by.length];
const type =
(use_table_schema && this._table_schema[title]) ||
this._schema[title] ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export function* format_tree_header(paths = [], row_headers, regularTable) {
path = path.slice(0, path.length - 1).fill("");
const formatted = format_cell.call(
this,
[row_headers[path.length - 1]],
row_headers[path.length - 1],
last,
plugins,
true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export function createDataListener() {
column.map((x) =>
format_cell.call(
this,
path_parts,
path_parts[this._config.split_by.length],
x,
regularTable[PRIVATE_PLUGIN_SYMBOL]
)
Expand Down
22 changes: 21 additions & 1 deletion packages/perspective-viewer-datagrid/test/js/superstore.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@
// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

import { test } from "@playwright/test";
import { run_standard_tests } from "@finos/perspective-test";
import {
compareContentsToSnapshot,
compareLightDOMContents,
run_standard_tests,
} from "@finos/perspective-test";

async function getDatagridContents(page) {
return await page.evaluate(async () => {
Expand Down Expand Up @@ -39,4 +43,20 @@ test.describe("Datagrid with superstore data set", () => {
});

run_standard_tests("perspective-viewer-datagrid", getDatagridContents);

test("Row headers are printed correctly", async ({ page }) => {
await page.evaluate(async () => {
await document.querySelector("perspective-viewer").restore({
plugin: "Datagrid",
group_by: ["Ship Date"],
split_by: ["Ship Mode"],
columns: ["Sales", "Quantity", "Discount", "Profit"],
});
});

compareContentsToSnapshot(
await getDatagridContents(page),
"row-headers-are-printed-correctly"
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ pub struct FilterColumnProps {
}

impl PartialEq for FilterColumnProps {
fn eq(&self, _rhs: &Self) -> bool {
false
// self.idx == other.idx && self.filter == other.filter
fn eq(&self, rhs: &Self) -> bool {
self.idx == rhs.idx && self.filter == rhs.filter && self.on_keydown == rhs.on_keydown
}
}

Expand Down Expand Up @@ -219,13 +218,15 @@ impl FilterColumnProps {
};

if let Some(input) = filter_input {
filter_column.2 = input;
let update = ViewConfigUpdate {
filter: Some(filter),
..ViewConfigUpdate::default()
};
if input != filter_column.2 {
filter_column.2 = input;
let update = ViewConfigUpdate {
filter: Some(filter),
..ViewConfigUpdate::default()
};

ApiFuture::spawn(self.update_and_render(update));
ApiFuture::spawn(self.update_and_render(update));
}
}
}
}
Expand Down
40 changes: 38 additions & 2 deletions rust/perspective-viewer/test/js/regressions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@
// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃
// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

import { test } from "@playwright/test";
import { compareContentsToSnapshot } from "@finos/perspective-test";
import { test, expect } from "@playwright/test";
import {
compareContentsToSnapshot,
shadow_type,
} from "@finos/perspective-test";

async function get_contents(page) {
return await page.evaluate(async () => {
Expand Down Expand Up @@ -57,4 +60,37 @@ test.describe("Regression tests", () => {
"regressions-not_in-filter-works-correctly.txt",
]);
});

test("Numeric filter input does not trigger render on trailing zeroes", async ({
page,
}) => {
await page.evaluate(async () => {
const viewer = document.querySelector("perspective-viewer");
await viewer.restore({
filter: [["Sales", ">", 1.1]],
settings: true,
});
});

// await new Promise((x) => setTimeout(x, 10000));

await shadow_type(
page,
"0001",
true,
"perspective-viewer",
"input.num-filter"
);

const value = await page.evaluate(async () => {
const viewer = document.querySelector("perspective-viewer");
return viewer.shadowRoot.querySelector("input.num-filter").value;
});

expect(value).toEqual("1.10001");
const contents = await get_contents(page);
await compareContentsToSnapshot(contents, [
"numeric-filter-input-does-not-trigger-render-on-trailing-zeroes.txt",
]);
});
});
Binary file modified tools/perspective-test/results.tar.gz
Binary file not shown.

0 comments on commit 285a76c

Please sign in to comment.