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

#5717: swap native select widget for searchable select widget in Form Builder #5750

Merged
merged 21 commits into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
909 changes: 619 additions & 290 deletions package-lock.json

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"@atlaskit/tree": "^8.8.4",
"@cfworker/json-schema": "^1.8.1",
"@datadog/browser-rum": "^4.42.2",
"@emotion/react": "^11.11.0",
"@floating-ui/dom": "^1.2.8",
"@fortawesome/fontawesome-svg-core": "1.2.36",
"@fortawesome/free-brands-svg-icons": "^5.15.4",
Expand Down Expand Up @@ -124,7 +125,7 @@
"react-router-dom": "^5.3.4",
"react-select": "^5.7.3",
"react-select-virtualized": "^5.1.0",
"react-shadow-root": "^6.1.0",
"react-shadow": "^20.0.0",
"react-spinners": "^0.13.0",
"react-table": "^7.7.0",
"react-transition-group": "^4.4.2",
Expand Down
2 changes: 2 additions & 0 deletions src/blocks/renderers/customForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ import {
type ComponentRef,
} from "@/types/runtimeTypes";
import { Renderer } from "@/types/blocks/rendererTypes";
import RjsfSelectWidget from "@/components/formBuilder/RjsfSelectWidget";

const fields = {
DescriptionField,
};
const uiWidgets = {
imageCrop: ImageCropWidget,
SelectWidget: RjsfSelectWidget,
};

interface DatabaseResult {
Expand Down
27 changes: 12 additions & 15 deletions src/blocks/renderers/documentView/DocumentView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import { buildDocumentBranch } from "@/components/documentBuilder/documentTree";
import React from "react";
import ReactShadowRoot from "react-shadow-root";
import EmotionShadowRoot from "react-shadow/emotion";
import bootstrap from "bootstrap/dist/css/bootstrap.min.css?loadAsUrl";
import { type DocumentViewProps } from "./DocumentViewProps";
import DocumentContext from "@/components/documentBuilder/render/DocumentContext";
Expand All @@ -42,21 +42,18 @@ const DocumentView: React.FC<DocumentViewProps> = ({

return (
// Wrap in a React context provider that passes BlockOptions down to any embedded bricks
// ReactShadowRoot needs to be inside an HTMLElement to attach to something
<DocumentContext.Provider value={{ options, meta, onAction }}>
<div className="h-100">
<ReactShadowRoot>
<Stylesheets href={bootstrap}>
{body.map((documentElement, index) => {
const { Component, props } = buildDocumentBranch(
documentElement,
{ staticId: joinPathParts("body", "children"), branches: [] }
);
return <Component key={index} {...props} />;
})}
</Stylesheets>
</ReactShadowRoot>
</div>
<EmotionShadowRoot.div className="h-100">
<Stylesheets href={bootstrap}>
{body.map((documentElement, index) => {
const { Component, props } = buildDocumentBranch(documentElement, {
staticId: joinPathParts("body", "children"),
branches: [],
});
return <Component key={index} {...props} />;
})}
</Stylesheets>
</EmotionShadowRoot.div>
</DocumentContext.Provider>
);
};
Expand Down
2 changes: 2 additions & 0 deletions src/blocks/transformers/ephemeralForm/EphemeralForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ import DescriptionField from "@/components/formBuilder/DescriptionField";
import FieldTemplate from "@/components/formBuilder/FieldTemplate";
import reportError from "@/telemetry/reportError";
import ErrorBoundary from "@/components/ErrorBoundary";
import RjsfSelectWidget from "@/components/formBuilder/RjsfSelectWidget";

const fields = {
DescriptionField,
};
const uiWidgets = {
imageCrop: ImageCropWidget,
SelectWidget: RjsfSelectWidget,
};

const ModalLayout: React.FC = ({ children }) => (
Expand Down
84 changes: 63 additions & 21 deletions src/components/formBuilder/FormBuilder.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,33 @@ async function renameField(newName: string) {
await waitForEffect();
}

/**
* Utility function to get react-select option labels
*/
function getAllReactSelectOptionLabels(
reactSelectContainer: HTMLElement
): string[] {
const reactSelectOptionQueryString = '[id^="react-select-"][id*="-option-"]';

const options = [];

for (const item of reactSelectContainer.querySelectorAll(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is fine for now as long as it works and we have test coverage and the tests are passing.

In the future, might be nice to try and cut these over to use React Testing Library more specific assertions/matchers, as opposed to using the querySelector escape hatch and asserting null or not.

reactSelectOptionQueryString
)) {
options.push(item.textContent);
}

return options;
}

/**
* React-select does not allow let you pass testids through
* and the id is duplicate at the time of writing this, so we use
* a wrapper with the testid and get the first div as the container.
*/
const getReactSelectContainer = (): HTMLElement =>
screen.getByTestId("formbuilder-select-wrapper").querySelector("div");

describe("Dropdown field", () => {
async function addOption() {
// Add a text option
Expand Down Expand Up @@ -106,29 +133,38 @@ describe("Dropdown field", () => {

test("doesn't fail when field type changed to Dropdown", async () => {
// Expect the dropdown rendered in the preview
expect(
rendered.container.querySelector(`select#root_${defaultFieldName}`)
).not.toBeNull();
expect(getReactSelectContainer()).not.toBeNull();
});

test("can add an option", async () => {
const selectContainer = getReactSelectContainer();
selectEvent.openMenu(selectContainer);

// Ensure no existing options first
expect(getAllReactSelectOptionLabels(selectContainer)).toBeArrayOfSize(0);

// Add option
await addOption();

// Expect the dropdown option rendered in the preview
expect(
screen.queryByRole("option", { name: "Test option" })
).not.toBeNull();
// Ensure new option's added
expect(getAllReactSelectOptionLabels(selectContainer)).toContain(
"Test option"
);
});

test("can use @var", async () => {
await setVarValue();
const container = getReactSelectContainer();
selectEvent.openMenu(container);

// Expect the dropdown option rendered in the preview
expect(screen.queryByRole("option", { name: "@data" })).not.toBeNull();
await setVarValue();
expect(getAllReactSelectOptionLabels(container)).toContain("@data");
});

describe("can be switched to Dropdown with labels", () => {
test("with items", async () => {
const selectContainer = getReactSelectContainer();
selectEvent.openMenu(selectContainer);

await addOption();

// Switch to Dropdown widget
Expand All @@ -153,9 +189,9 @@ describe("Dropdown field", () => {
expect(firstOptionTitleInput).toHaveValue("");

// Expect the dropdown option rendered in the preview
expect(
screen.queryByRole("option", { name: "Test option" })
).not.toBeNull();
expect(getAllReactSelectOptionLabels(selectContainer)).toContain(
"Test option"
);
});

test("with @var", async () => {
Expand Down Expand Up @@ -222,29 +258,35 @@ describe("Dropdown with labels field", () => {
});
test("doesn't fail when field type changed to Dropdown with labels", async () => {
// Expect the dropdown rendered in the preview
expect(
rendered.container.querySelector(`select#root_${defaultFieldName}`)
).not.toBeNull();
expect(getReactSelectContainer()).not.toBeNull();
});

test("can add an option", async () => {
const selectContainer = getReactSelectContainer();
selectEvent.openMenu(selectContainer);
await addOption();

// Validate the rendered option
const optionElement = screen.queryByRole("option", { name: "Test option" });
expect(optionElement).not.toBeNull();
expect(optionElement).toHaveValue("1");
expect(getAllReactSelectOptionLabels(selectContainer)).toContain(
"Test option"
);
});

test("can use @var in Dropdown", async () => {
const selectContainer = getReactSelectContainer();
selectEvent.openMenu(selectContainer);

await setVarValue();

// Expect the dropdown option rendered in the preview
expect(screen.queryByRole("option", { name: "@data" })).not.toBeNull();
expect(getAllReactSelectOptionLabels(selectContainer)).toContain("@data");
});

describe("can be switched to regular Dropdown", () => {
test("with items", async () => {
const selectContainer = getReactSelectContainer();
selectEvent.openMenu(selectContainer);

await addOption();

// Switch to Dropdown widget
Expand All @@ -262,7 +304,7 @@ describe("Dropdown with labels field", () => {
expect(firstOptionInput).toHaveValue("1");

// Expect the dropdown option rendered in the preview
expect(screen.queryByRole("option", { name: "1" })).not.toBeNull();
expect(getAllReactSelectOptionLabels(selectContainer)).toContain("1");
});

test("with @var", async () => {
Expand Down
86 changes: 86 additions & 0 deletions src/components/formBuilder/RjsfSelectWidget.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* Copyright (C) 2023 PixieBrix, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import React from "react";
import { type WidgetProps } from "@rjsf/core";
import Select from "react-select";
import { FormLabel, FormGroup } from "react-bootstrap";

type OptionType = { label: string; value: string };

const defaultOption: OptionType[] = [];

const RjsfSelectWidget: React.FC<WidgetProps> = ({
schema,
id,
options,
value,
required,
disabled,
readonly,
onChange,
onBlur,
onFocus,
rawErrors,
label,
multiple,
}) => {
const _onChange = (option: OptionType | null) => {
onChange(option ? option.value : "");
};

const _onBlur = () => {
onBlur(id, value);
};

const _onFocus = () => {
onFocus(id, value);
};

// Check to ensure enumOptions is an array
// RJSF seems to occasionally set enumOptions as FALSE. Maybe related to async options:
// https://github.com/rjsf-team/react-jsonschema-form/issues/415
const enumOptions = (options.enumOptions || defaultOption) as OptionType[];

const selectOptions =
enumOptions.map(({ value, label }) => ({
value,
label,
})) ?? [];

return (
<FormGroup>
<FormLabel className={rawErrors?.length > 0 ? "text-danger" : ""}>
{label || schema.title}
{(label || schema.title) && required ? "*" : null}
</FormLabel>
<div data-testid="formbuilder-select-wrapper">
<Select
id={id}
options={selectOptions}
isDisabled={disabled || readonly}
isMulti={multiple}
value={selectOptions.find((option) => option.value === value)}
onChange={_onChange}
onBlur={_onBlur}
onFocus={_onFocus}
/>
</div>
</FormGroup>
);
};

export default RjsfSelectWidget;
14 changes: 6 additions & 8 deletions src/components/formBuilder/preview/FormPreview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import { unwrapTemplateExpressions } from "@/components/fields/fieldUtils";
import ImageCropWidgetPreview from "@/components/formBuilder/preview/ImageCropWidgetPreview";
import DescriptionField from "@/components/formBuilder/DescriptionField";
import FieldTemplate from "@/components/formBuilder/FieldTemplate";
import SelectWidgetPreview from "./SelectWidgetPreview";
import RjsfSelectWidget from "@/components/formBuilder/RjsfSelectWidget";
import FormPreviewSchemaField from "./FormPreviewSchemaField";
import databaseSchema from "@schemas/database.json";
import googleSheetSchema from "@schemas/googleSheetId.json";
Expand Down Expand Up @@ -88,12 +88,10 @@ const FormPreview: React.FC<FormPreviewProps> = ({

if (property.format === "preview") {
// Intentionally setting a string value, not an array. @see FormPreviewSchemaField for details
// @ts-expect-error -- intentionally assigning to a string
property.enum = `[Mod Name] - ${activeField} database`;
property.enum = [`[Mod Name] - ${activeField} database`];
} else {
// Intentionally setting a string value, not an array. @see FormPreviewSchemaField for details
// @ts-expect-error -- intentionally assigning to a string
property.enum = "Select...";
Comment on lines -91 to -96
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BLoe here's my slightly-less-hacky solution. It should work the same way except it doesn't disable the field in the preview.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool, makes sense

property.enum = ["Select..."];
}

delete property.$ref;
Expand Down Expand Up @@ -179,9 +177,9 @@ const FormPreview: React.FC<FormPreviewProps> = ({

const widgets = {
imageCrop: ImageCropWidgetPreview,
database: SelectWidgetPreview,
SelectWidget: SelectWidgetPreview,
googleSheet: SelectWidgetPreview,
database: RjsfSelectWidget,
SelectWidget: RjsfSelectWidget,
googleSheet: RjsfSelectWidget,
};

return (
Expand Down