Skip to content

Commit

Permalink
make sure combobox respects defaultSelected (#1131)
Browse files Browse the repository at this point in the history
* make sure combobox respects defaultSelected

* additional ComboBox fixes, multi select, cypress tests

* omit cypress config from eslint checks

* fix bad typescript typing, remove unused code

* additional type fixes
  • Loading branch information
heswell committed Jan 18, 2024
1 parent 75a6b1b commit 2cf73b8
Show file tree
Hide file tree
Showing 23 changed files with 725 additions and 106 deletions.
2 changes: 2 additions & 0 deletions vuu-ui/.eslintignore
Expand Up @@ -19,3 +19,5 @@ showcase/dist
target
packages/shell/src/feature/Feature.jsx
**/inlined-worker.js
cypress.config.ts
cypress
3 changes: 2 additions & 1 deletion vuu-ui/cypress/support/component.ts
Expand Up @@ -4,7 +4,8 @@ import "./component/assertions";
import "./component/commands";

import "./component/cypress.css";
import "./component/index.css";
import "@finos/vuu-theme/index.css";
import "@finos/vuu-icons/index.css";

beforeEach(() => {
cy.window({ log: false }).focus({ log: false });
Expand Down
109 changes: 109 additions & 0 deletions vuu-ui/cypress/support/component/assertions.ts
@@ -1,5 +1,13 @@
import AssertionStatic = Chai.AssertionStatic;
import ChaiPlugin = Chai.ChaiPlugin;
import { prettyDOM } from "@testing-library/dom";

function elementToString(element: Element | null | undefined) {
if (typeof element?.nodeType === "number") {
return prettyDOM(element, undefined, { highlight: true, maxDepth: 1 });
}
return String(element);
}

// Must be declared global to be detected by typescript (allows import/export)
declare global {
Expand Down Expand Up @@ -92,6 +100,42 @@ declare global {
* */
(chainer: "not.be.focusVisible"): Chainable<Subject>;
(chainer: "not.have.focusVisible"): Chainable<Subject>;
/**
* Checks if the element is in the viewport.
*
* @example
```
cy.findByRole('option).should('be.inTheViewport')
```
* */
(chainer: "be.inTheViewport"): Chainable<Subject>;
/**
* Checks if the element is not in the viewport.
*
* @example
```
cy.findByRole('option).should('not.be.inTheViewport')
```
* */
(chainer: "not.be.inTheViewport"): Chainable<Subject>;
/**
* Checks if the element is the active descendant.
*
* @example
```
cy.findByRole('option).should('be.activeDescendant')
```
* */
(chainer: "be.activeDescendant"): Chainable<Subject>;
/**
* Checks if the element is not the active descendant.
*
* @example
```
cy.findByRole('option).should('not.be.activeDescendant')
```
* */
(chainer: "not.be.activeDescendant"): Chainable<Subject>;
}
}
}
Expand Down Expand Up @@ -192,4 +236,69 @@ const hasAriaSelected: ChaiPlugin = (_chai) => {
// registers our assertion function "isHighlighted" with Chai
chai.use(hasAriaSelected);

/**
* Checks if the element is in the viewport
*
* @example
* cy.findByRole('option).should('be.inTheViewport')
*/
const isInTheViewport: ChaiPlugin = (_chai, utils) => {
function assertIsInTheViewport(this: AssertionStatic) {
const root = this._obj.get(0);
// make sure it's an Element
new _chai.Assertion(
root.nodeType,
`Expected an Element but got '${String(root)}'`
).to.equal(1);

const viewportHeight = Cypress.config(`viewportHeight`);
const rect = root.getBoundingClientRect();

this.assert(
!(rect.bottom < 0 || rect.top - viewportHeight >= 0),
`expected \n${elementToString(root)} to be in the viewport.`,
`expected \n${elementToString(root)} to not be in the viewport`,
null
);
}

_chai.Assertion.addMethod("inTheViewport", assertIsInTheViewport);
};

// registers our assertion function "isInTheViewport" with Chai
chai.use(isInTheViewport);

/**
* Checks if the element is in the viewport
*
* @example
* cy.findByRole('option).should('be.activeDescendant')
*/
const isActiveDescendant: ChaiPlugin = (_chai) => {
function assertIsActiveDescendant(this: AssertionStatic) {
// make sure it's an Element
const root = this._obj.get(0);
// make sure it's an Element
new _chai.Assertion(
root.nodeType,
`Expected an Element but got '${String(root)}'`
).to.equal(1);

const id = root.id;
cy.focused({ log: false }).then(($focused) => {
this.assert(
$focused.attr("aria-activedescendant") === id,
"expected #{this} to be #{exp}",
"expected #{this} not to be #{exp}",
"active descendant"
);
});
}

_chai.Assertion.addMethod("activeDescendant", assertIsActiveDescendant);
};

// registers our assertion function "isFocused" with Chai
chai.use(isActiveDescendant);

export {};
2 changes: 1 addition & 1 deletion vuu-ui/cypress/support/component/commands.tsx
Expand Up @@ -99,7 +99,7 @@ Cypress.Commands.add(

Cypress.Commands.add("mount", function (children, options) {
return cypressMount(
<ThemeProvider density="high" theme="vuu-theme">
<ThemeProvider density="high" theme="vuu">
{children},
</ThemeProvider>,
options
Expand Down
1 change: 0 additions & 1 deletion vuu-ui/cypress/support/component/component-index.html
Expand Up @@ -5,7 +5,6 @@
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<meta name="viewport" content="width=device-width,initial-scale=1.0" />
<title>Components App</title>
<link rel="stylesheet" href=""https://fonts.googleapis.com/css?family=Nunito+Sans:300,400,500,600,700&display=swap" />
</head>
<body>
<div data-cy-root class="vuu-theme vuu-density-high" data-mode="light"></div>
Expand Down
2 changes: 0 additions & 2 deletions vuu-ui/cypress/support/component/index.css

This file was deleted.

7 changes: 0 additions & 7 deletions vuu-ui/cypress/support/component/index.css.map

This file was deleted.

3 changes: 0 additions & 3 deletions vuu-ui/cypress/tsconfig.json
Expand Up @@ -2,9 +2,6 @@
"extends": "../tsconfig.json",
"compilerOptions": {
"types": ["cypress", "@testing-library/cypress"],
"typeRoots": [
"./support"
]
},
"include": ["**/*.ts", "**/*.tsx", "../cypress.d.ts"]
}
1 change: 1 addition & 0 deletions vuu-ui/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions vuu-ui/package.json
Expand Up @@ -61,6 +61,7 @@
"@heswell/component-anatomy": "0.0.5",
"@lezer/generator": "^1.2.2",
"@testing-library/cypress": "10.0.1",
"@testing-library/dom": "^9.0.0",
"@types/jsdom": "^21.1.2",
"@types/testing-library__cypress": "5.0.13",
"@typescript-eslint/eslint-plugin": "^5.41.0",
Expand Down
32 changes: 27 additions & 5 deletions vuu-ui/packages/vuu-filters/src/filter-clause/ExpandoCombobox.tsx
Expand Up @@ -27,8 +27,10 @@ const NO_INPUT_PROPS = {};
export interface ExpandoComboboxProps<
Item = string,
S extends SelectionStrategy = "default"
> extends ComboBoxProps<Item, S> {
> extends Omit<ComboBoxProps<Item, S>, "itemToString" | "value"> {
itemToString?: (item: unknown) => string;
onInputChange?: (evt: FormEvent<HTMLInputElement>) => void;
value?: string | string[];
}

export const ExpandoCombobox = forwardRef(function ExpandoCombobox<
Expand All @@ -52,8 +54,8 @@ export const ExpandoCombobox = forwardRef(function ExpandoCombobox<
const { itemToString = defaultToString } = props;
const initialValue = useRef(value);

const itemsToString = useCallback(
(items: Item[]) => {
const itemsToString = useCallback<<I = Item>(items: I[]) => string>(
(items) => {
const [first, ...rest] = items;
if (rest.length) {
return `${itemToString(first)} + ${rest.length}`;
Expand All @@ -78,7 +80,6 @@ export const ExpandoCombobox = forwardRef(function ExpandoCombobox<
}, []);

const [InputProps, ListProps] = useMemo<
// [ComboBoxProps["InputProps"], ComboBoxProps["ListProps"]]
[ComboBoxProps["InputProps"], any]
>(() => {
const { inputProps, ...restInputProps } = InputPropsProp;
Expand Down Expand Up @@ -126,6 +127,22 @@ export const ExpandoCombobox = forwardRef(function ExpandoCombobox<
[itemToString, onSelectionChange]
);

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const getDefaultSelected = (): any => {
if (initialValue.current === undefined) {
return undefined;
} else if (Array.isArray(initialValue.current)) {
return props.source?.filter((item) =>
initialValue.current.includes(itemToString(item))
);
} else {
return props.source?.find(
(item) => itemToString(item) === initialValue.current
);
}
};

const popupProps = {
minWidth: "fit-content",
};
Expand All @@ -139,7 +156,12 @@ export const ExpandoCombobox = forwardRef(function ExpandoCombobox<
<ComboBox<Item, S>
{...props}
PopupProps={popupProps}
defaultValue={initialValue.current}
defaultSelected={getDefaultSelected()}
defaultValue={
Array.isArray(initialValue.current)
? itemsToString<string>(initialValue.current)
: initialValue.current
}
fullWidth
ListProps={ListProps}
InputProps={InputProps}
Expand Down
@@ -1,8 +1,9 @@
import { TableSchema } from "@finos/vuu-data-types";
import { SuggestionFetcher } from "@finos/vuu-data-react";
import { ColumnDescriptor } from "@finos/vuu-table-types";
import { TableSchema } from "@finos/vuu-data-types";
import { FilterClause } from "@finos/vuu-filter-types";
import { ColumnDescriptor } from "@finos/vuu-table-types";
import { CloseReason } from "@finos/vuu-ui-controls";
import { Button } from "@salt-ds/core";
import cx from "clsx";
import { HTMLAttributes, useCallback } from "react";
import { ExpandoCombobox } from "./ExpandoCombobox";
Expand All @@ -15,7 +16,6 @@ import {
} from "./useFilterClauseEditor";

import "./FilterClauseEditor.css";
import { Button } from "@salt-ds/core";

export interface FilterClauseEditorProps
extends Omit<HTMLAttributes<HTMLDivElement>, "onChange"> {
Expand Down Expand Up @@ -56,7 +56,6 @@ export const FilterClauseEditor = ({
operatorRef,
selectedColumn,
value,
valueRef,
} = useFilterClauseEditor({
filterClause,
onCancel,
Expand All @@ -81,10 +80,9 @@ export const FilterClauseEditor = ({
onDeselect={onDeselectValue}
onInputComplete={onChangeValue}
operator={operator}
ref={valueRef}
suggestionProvider={suggestionProvider}
table={table}
value={value as string}
value={value as string | string[]}
/>
);
case "int":
Expand All @@ -98,7 +96,6 @@ export const FilterClauseEditor = ({
filterClause={filterClause}
onInputComplete={onChangeValue}
operator={operator}
ref={valueRef}
/>
);
case undefined:
Expand All @@ -113,8 +110,8 @@ export const FilterClauseEditor = ({
operator,
InputProps,
filterClause,
onDeselectValue,
onChangeValue,
valueRef,
suggestionProvider,
table,
value,
Expand All @@ -128,7 +125,7 @@ export const FilterClauseEditor = ({
className={cx(`${classBase}Field`, `${classBase}Column`)}
data-field="column"
initialHighlightedIndex={0}
itemToString={(column) => column.name}
itemToString={(column) => (column as ColumnDescriptor).name}
onListItemSelect={onColumnSelect}
ref={columnRef}
source={columns}
Expand Down
23 changes: 17 additions & 6 deletions vuu-ui/packages/vuu-filters/src/filter-clause/TextInput.tsx
Expand Up @@ -32,7 +32,7 @@ export interface TextInputProps
ref: RefObject<HTMLDivElement>;
operator: string;
suggestionProvider?: () => SuggestionFetcher;
value: string;
value: string | string[];
}

const NO_DATA_MATCH = ["No matching data"];
Expand All @@ -52,7 +52,17 @@ export const TextInput = forwardRef(function TextInput(
}: TextInputProps,
forwardedRef: ForwardedRef<HTMLDivElement>
) {
const [valueInputValue, setValueInputValue] = useState(value ?? "");
const isMultiValue = operator === "in";

// If we have a multiselect text value which we are editing, this will render
// a comma delimited list of the selected values. That is not what we display
// by default when using a multiselect combo. Its not a huge problem - as soon
// as user focuses this component and we display dropdown, input text is cleared
// (so user can type to filter list) until dropdown closes again. <ight need to
// revisit.
const [valueInputValue, setValueInputValue] = useState(
value?.toString() ?? ""
);
const [typeaheadValues, setTypeaheadValues] = useState<string[]>([]);

const getSuggestions = suggestionProvider();
Expand All @@ -69,9 +79,10 @@ export const TextInput = forwardRef(function TextInput(

useEffect(() => {
if (table) {
const params: TypeaheadParams = valueInputValue
? [table, column.name, valueInputValue]
: [table, column.name];
const params: TypeaheadParams =
valueInputValue && !isMultiValue
? [table, column.name, valueInputValue]
: [table, column.name];
getSuggestions(params)
.then((suggestions) => {
if (suggestions.length === 0 && valueInputValue) {
Expand All @@ -84,7 +95,7 @@ export const TextInput = forwardRef(function TextInput(
console.error("Error getting suggestions", err);
});
}
}, [table, column, valueInputValue, getSuggestions]);
}, [table, column, valueInputValue, getSuggestions, isMultiValue]);

const handleInputChange = useCallback((evt: FormEvent<HTMLInputElement>) => {
const { value } = evt.target as HTMLInputElement;
Expand Down
Expand Up @@ -174,7 +174,6 @@ export const useFilterClauseEditor = ({
}: FilterClauseEditorHookProps) => {
const columnRef = useRef<HTMLDivElement>(null);
const operatorRef = useRef<HTMLDivElement>(null);
const valueRef = useRef<HTMLDivElement>(null);

const [selectedColumn, setSelectedColumn] = useState<
ColumnDescriptor | undefined
Expand All @@ -183,12 +182,6 @@ export const useFilterClauseEditor = ({
filterClause.op
);

const findColumn = useCallback(
(columnName: string) =>
tableSchema.columns.find((col) => col.name === columnName),
[tableSchema.columns]
);

const setOperator = useCallback((op) => {
_setOperator(op);
}, []);
Expand Down Expand Up @@ -359,6 +352,5 @@ export const useFilterClauseEditor = ({
operatorRef,
selectedColumn,
value,
valueRef,
};
};

0 comments on commit 2cf73b8

Please sign in to comment.