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.

Sign up for GitHub

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

bootstrap-4 TextWidget wrappers now pull from registry, add rootSchema to Registry, Fix FieldProps.onFocus type to match WidgetProps #2519

Merged
merged 4 commits into from
Aug 26, 2021
Merged

bootstrap-4 TextWidget wrappers now pull from registry, add rootSchema to Registry, Fix FieldProps.onFocus type to match WidgetProps #2519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1f7c81d
In all the bootstrap-4 `TextWidget` wrappers, rather than directly im…
heath-freenome Aug 17, 2021
c6c7df6
Merge branch 'master' into bootstrap-4-pulls-TextWidget-from-registry
heath-freenome Aug 17, 2021
8cf72b8
Update index.d.ts
heath-freenome Aug 17, 2021
d1379b8
Update index.d.ts
heath-freenome Aug 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/bootstrap-4/src/ArrayFieldTemplate/ArrayFieldTemplate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ const { isMultiSelect, getDefaultRegistry } = utils;
const ArrayFieldTemplate = (props: ArrayFieldTemplateProps) => {
const { schema, registry = getDefaultRegistry() } = props;

// TODO: update types so we don't have to cast registry as any
if (isMultiSelect(schema, (registry as any).rootSchema)) {
if (isMultiSelect(schema, registry.rootSchema)) {
return <DefaultFixedArrayFieldTemplate {...props} />;
} else {
return <DefaultNormalArrayFieldTemplate {...props} />;
Expand Down
6 changes: 4 additions & 2 deletions packages/bootstrap-4/src/ColorWidget/ColorWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React from "react";
import TextWidget, { TextWidgetProps } from "../TextWidget";
import { WidgetProps } from '@rjsf/core';

const ColorWidget = (props: TextWidgetProps) => {
const ColorWidget = (props: WidgetProps) => {
const { registry } = props;
const { TextWidget } = registry.widgets;
return <TextWidget {...props} type="color" />;
};

Expand Down
9 changes: 5 additions & 4 deletions packages/bootstrap-4/src/DateTimeWidget/DateTimeWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import React from "react";
import { utils } from "@rjsf/core";
import TextWidget, { TextWidgetProps } from "../TextWidget";
import { utils, WidgetProps } from "@rjsf/core";

const { localToUTC, utcToLocal } = utils;

const DateTimeWidget = (props: TextWidgetProps) => {
const DateTimeWidget = (props: WidgetProps) => {
const { registry } = props;
const { TextWidget } = registry.widgets;
const value = utcToLocal(props.value);
const onChange = (value: any) => {
props.onChange(localToUTC(value));
};

return (
<TextWidget
<TextWidget
{...props}
type="datetime-local"
value={value}
Expand Down
10 changes: 6 additions & 4 deletions packages/bootstrap-4/src/DateWidget/DateWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import React from "react";
import TextWidget, { TextWidgetProps } from "../TextWidget";
import { WidgetProps } from '@rjsf/core';

const DateWidget = (props: TextWidgetProps) => {
const DateWidget = (props: WidgetProps) => {
const { registry } = props;
const { TextWidget } = registry.widgets;
return (
<TextWidget
{...props}
{...props}
type="date"
/>
);
};

export default DateWidget;
export default DateWidget;
6 changes: 4 additions & 2 deletions packages/bootstrap-4/src/EmailWidget/EmailWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React from "react";
import TextWidget, { TextWidgetProps } from "../TextWidget";
import { WidgetProps } from '@rjsf/core';

const EmailWidget = (props: TextWidgetProps) => {
const EmailWidget = (props: WidgetProps) => {
const { registry } = props;
const { TextWidget } = registry.widgets;
return <TextWidget {...props} type="email" />;
};

Expand Down
6 changes: 4 additions & 2 deletions packages/bootstrap-4/src/FileWidget/FileWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React from "react";
import TextWidget, { TextWidgetProps } from "../TextWidget";
import { WidgetProps } from '@rjsf/core';

const FileWidget = (props: TextWidgetProps) => {
const FileWidget = (props: WidgetProps) => {
const { registry } = props;
const { TextWidget } = registry.widgets;
return <TextWidget {...props} type="file"/>;
};

Expand Down
8 changes: 2 additions & 6 deletions packages/bootstrap-4/src/TextWidget/TextWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ import Form from "react-bootstrap/Form";

import { WidgetProps } from "@rjsf/core";

export interface TextWidgetProps extends WidgetProps {
type?: string;
}

const TextWidget = ({
id,
placeholder,
Expand All @@ -25,7 +21,7 @@ const TextWidget = ({
schema,
rawErrors = [],

}: TextWidgetProps) => {
}: WidgetProps) => {
const _onChange = ({
target: { value },
}: React.ChangeEvent<HTMLInputElement>) =>
Expand All @@ -36,7 +32,7 @@ const TextWidget = ({
target: { value },
}: React.FocusEvent<HTMLInputElement>) => onFocus(id, value);
const inputType = (type || schema.type) === 'string' ? 'text' : `${type || schema.type}`
Copy link
Member

Choose a reason for hiding this comment

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

The one thing remaining is that I'm a bit confused as to why TextWidget uses the type prop, but only in the bootstrap-4 theme. Could you point me to the code that actually passes the type prop? And would it be better / equivalent / more consistent to change this line of code to not use type, in favor of something like this (https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/fluent-ui/src/TextWidget/TextWidget.tsx#L73)?

Copy link
Member Author

@heath-freenome heath-freenome Aug 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The type prop is also used in the @material-ui theme... but the TextWidgetProps there have been expanded to include the @material-ui TextField props, so passing type in works there (sadly I spent a decent amount of time trying to apply a similar solution to bootstrap-4 with no success :( ). See https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/src/TextWidget/TextWidget.tsx#L11. I'm not against doing what fluent-ui does with type for bootstrap-4 if that feels more comfortable for you.

Copy link
Member Author

@heath-freenome heath-freenome Aug 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I am curious if you feel like the WidgetProps should be restricted to only those current props defined on it, unlike the FieldProps which has the catch-all "other props" [prop: string]: any; defined on it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly, with the change to WidgetProps that I'm proposing, I could probably remove the override that material-ui has since the catch-all would do the same thing. Heck, most of the code between the three packages for the TextWidget overrides could probably be standardized into the same exact code (most of which very closely matches how core does it)... which makes me wonder whether it makes sense to do a bit more work in core to use TextWidget rather than BaseInput... or make all three Typescript implementations simple provide their TextWidget implementation as the BaseInput and remove all the overrides that are simply the override the "type" since thats what core already does.

Copy link
Member

Choose a reason for hiding this comment

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

I am curious if you feel like the WidgetProps should be restricted to only those current props defined on it, unlike the FieldProps which has the catch-all "other props" [prop: string]: any; defined on it?

I think using the catch-all is OK for this PR.

Honestly, with the change to WidgetProps that I'm proposing, I could probably remove the override that material-ui has since the catch-all would do the same thing. Heck, most of the code between the three packages for the TextWidget overrides could probably be standardized into the same exact code (most of which very closely matches how core does it)... which makes me wonder whether it makes sense to do a bit more work in core to use TextWidget rather than BaseInput... or make all three Typescript implementations simple provide their TextWidget implementation as the BaseInput and remove all the overrides that are simply the override the "type" since thats what core already does.

Yeah, if you'd like to do additional refactoring, that would also be good. Let's not do it in this PR, though, maybe in a separate PR if you are interested in doing it.


// const classNames = [rawErrors.length > 0 ? "is-invalid" : "", type === 'file' ? 'custom-file-label': ""]
return (
<Form.Group className="mb-0">
Expand Down
6 changes: 4 additions & 2 deletions packages/bootstrap-4/src/URLWidget/URLWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React from "react";
import TextWidget, { TextWidgetProps } from "../TextWidget";
import { WidgetProps } from '@rjsf/core';

const URLWidget = (props: TextWidgetProps) => {
const URLWidget = (props: WidgetProps) => {
const { registry } = props;
const { TextWidget } = registry.widgets;
return <TextWidget {...props} type="url" />;
Copy link
Member Author

Choose a reason for hiding this comment

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

@epicfaace This is one place where type is passed... material-ui uses the same exact pattern. fluent-ui chose to put type into the ui:options.props and then extracts it on the line above what you mentioned, filters out non-fluent props and spreads it over the default type that is being extracted on the line you mentioned.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, this makes sense now

};

Expand Down
5 changes: 4 additions & 1 deletion packages/bootstrap-4/test/helpers/createMocks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { WidgetProps } from "@rjsf/core";
import { JSONSchema7 } from "json-schema";

import TextWidget from "../../src/TextWidget/TextWidget";

export const mockSchema: JSONSchema7 = {
type: "array",
items: {
Expand Down Expand Up @@ -33,9 +35,10 @@ export function makeWidgetMockProps(
placeholder: "",
registry: {
fields: {},
widgets: {},
widgets: { TextWidget },
formContext: {},
definitions: {},
rootSchema: {},
},
...props,
};
Expand Down
45 changes: 24 additions & 21 deletions packages/core/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,34 +115,37 @@ declare module '@rjsf/core' {
multiple: boolean;
rawErrors: string[];
registry: Registry;
[prop: string]: any; // Allow for other props
}

export type Widget = React.StatelessComponent<WidgetProps> | React.ComponentClass<WidgetProps>;

export interface Registry {
fields: { [name: string]: Field };
widgets: { [name: string]: Widget };
definitions: { [name: string]: any };
formContext: any;
fields: { [name: string]: Field };
widgets: { [name: string]: Widget };
definitions: { [name: string]: any };
formContext: any;
rootSchema: JSONSchema7;
}

export interface FieldProps<T = any>
extends Pick<React.HTMLAttributes<HTMLElement>, Exclude<keyof React.HTMLAttributes<HTMLElement>, 'onBlur'>> {
extends Pick<React.HTMLAttributes<HTMLElement>, Exclude<keyof React.HTMLAttributes<HTMLElement>, 'onBlur' | 'onFocus'>> {
schema: JSONSchema7;
uiSchema: UiSchema;
idSchema: IdSchema;
formData: T;
errorSchema: ErrorSchema;
onChange: (e: IChangeEvent<T> | any, es?: ErrorSchema) => any;
onBlur: (id: string, value: any) => void;
onFocus: (id: string, value: any) => void;
registry: Registry;
formContext: any;
autofocus: boolean;
disabled: boolean;
readonly: boolean;
required: boolean;
name: string;
[prop: string]: any;
[prop: string]: any; // Allow for other props
}

export type Field = React.StatelessComponent<FieldProps> | React.ComponentClass<FieldProps>;
Expand Down Expand Up @@ -171,7 +174,7 @@ declare module '@rjsf/core' {
onChange: (value: T) => void;
onKeyChange: (value: string) => () => void;
onDropPropertyClick: (value: string) => () => void;
registry: FieldProps['registry'];
registry: Registry;
};

export type ArrayFieldTemplateProps<T = any> = {
Expand Down Expand Up @@ -204,7 +207,7 @@ declare module '@rjsf/core' {
title: string;
formContext: any;
formData: T;
registry: FieldProps['registry'];
registry: Registry;
};

export type ObjectFieldTemplateProps<T = any> = {
Expand All @@ -228,7 +231,7 @@ declare module '@rjsf/core' {
idSchema: IdSchema;
formData: T;
formContext: any;
registry: FieldProps['registry'];
registry: Registry;
};

export type AjvError = {
Expand Down Expand Up @@ -293,7 +296,7 @@ declare module '@rjsf/core' {

export function canExpand(schema: JSONSchema7, uiSchema: UiSchema, formData: any): boolean;

export function getDefaultRegistry(): FieldProps['registry'];
export function getDefaultRegistry(): Registry;

export function getSchemaType(schema: JSONSchema7): string;

Expand All @@ -312,15 +315,15 @@ declare module '@rjsf/core' {
export function computeDefaults<T = any>(
schema: JSONSchema7,
parentDefaults: JSONSchema7['default'][],
definitions: FieldProps['registry']['definitions'],
definitions: Registry['definitions'],
rawFormData?: T,
includeUndefinedValues?: boolean,
): JSONSchema7['default'][];

export function getDefaultFormState<T = any>(
schema: JSONSchema7,
formData: T,
definitions?: FieldProps['registry']['definitions'],
definitions?: Registry['definitions'],
includeUndefinedValues?: boolean,
): T | JSONSchema7['default'][];

Expand All @@ -340,14 +343,14 @@ declare module '@rjsf/core' {

export function toConstant(schema: JSONSchema7): JSONSchema7Type | JSONSchema7['const'];

export function isSelect(_schema: JSONSchema7, definitions?: FieldProps['registry']['definitions']): boolean;
export function isSelect(_schema: JSONSchema7, definitions?: Registry['definitions']): boolean;

export function isMultiSelect(schema: JSONSchema7, definitions?: FieldProps['registry']['definitions']): boolean;
export function isMultiSelect(schema: JSONSchema7, definitions?: Registry['definitions']): boolean;

export function isFilesArray(
schema: JSONSchema7,
uiSchema: UiSchema,
definitions?: FieldProps['registry']['definitions'],
definitions?: Registry['definitions'],
): boolean;

export function isFixedItems(schema: JSONSchema7): boolean;
Expand All @@ -364,19 +367,19 @@ declare module '@rjsf/core' {

export function stubExistingAdditionalProperties<T = any>(
schema: JSONSchema7,
definitions?: FieldProps['registry']['definitions'],
definitions?: Registry['definitions'],
formData?: T,
): JSONSchema7;

export function resolveSchema<T = any>(
schema: JSONSchema7Definition,
definitions?: FieldProps['registry']['definitions'],
definitions?: Registry['definitions'],
formData?: T,
): JSONSchema7;

export function retrieveSchema<T = any>(
schema: JSONSchema7Definition,
definitions?: FieldProps['registry']['definitions'],
definitions?: Registry['definitions'],
formData?: T,
): JSONSchema7;

Expand All @@ -387,15 +390,15 @@ declare module '@rjsf/core' {
export function toIdSchema<T = any>(
schema: JSONSchema7Definition,
id: string,
definitions: FieldProps['registry']['definitions'],
definitions: Registry['definitions'],
formData?: T,
idPredix?: string,
): IdSchema | IdSchema[];

export function toPathSchema<T = any>(
schema: JSONSchema7Definition,
name: string | undefined,
definitions: FieldProps['registry']['definitions'],
definitions: Registry['definitions'],
formData?: T,
): PathSchema | PathSchema[];

Expand Down Expand Up @@ -433,7 +436,7 @@ declare module '@rjsf/core' {
export function getMatchingOption(
formData: any,
options: JSONSchema7[],
definitions: FieldProps['registry']['definitions'],
definitions: Registry['definitions'],
): number;

export function schemaRequiresTrueValue(schema: JSONSchema7): boolean;
Expand Down
3 changes: 1 addition & 2 deletions packages/fluent-ui/src/ArrayFieldTemplate/ArrayFieldTemplate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ const { isMultiSelect, getDefaultRegistry } = utils;
const ArrayFieldTemplate = (props: ArrayFieldTemplateProps) => {
const { schema, registry = getDefaultRegistry() } = props;

// TODO: update types so we don't have to cast registry as any
if (isMultiSelect(schema, (registry as any).rootSchema)) {
if (isMultiSelect(schema, registry.rootSchema)) {
return <DefaultFixedArrayFieldTemplate {...props} />;
} else {
return <DefaultNormalArrayFieldTemplate {...props} />;
Expand Down
3 changes: 1 addition & 2 deletions packages/material-ui/src/ArrayFieldTemplate/ArrayFieldTemplate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ const {
const ArrayFieldTemplate = (props: ArrayFieldTemplateProps) => {
const { schema, registry = getDefaultRegistry() } = props;

// TODO: update types so we don't have to cast registry as any
if (isMultiSelect(schema, (registry as any).rootSchema)) {
if (isMultiSelect(schema, registry.rootSchema)) {
return <DefaultFixedArrayFieldTemplate {...props} />;
} else {
return <DefaultNormalArrayFieldTemplate {...props} />;
Expand Down
1 change: 1 addition & 0 deletions packages/material-ui/test/UpDownWidget.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe("UpDownWidget", () => {
widgets: {},
definitions: {},
formContext: {},
rootSchema: {},
}
};
const tree = renderer.create(<UpDownWidget {...props} />).toJSON();
Expand Down