-
Notifications
You must be signed in to change notification settings - Fork 22
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
#7558: Improve document/form builder responsiveness #7976
Changes from all commits
98fc81a
23294b9
99c512f
a1ef3b2
67806d7
812d84c
6f176e8
472b940
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,8 @@ | |
import styles from "./FormBuilder.module.scss"; | ||
|
||
import React, { useState } from "react"; | ||
import FormEditor from "./edit/FormEditor"; | ||
import FormEditor from "@/components/formBuilder/edit/FormEditor"; | ||
import FormIntroFields from "@/components/formBuilder/edit/FormIntroFields"; | ||
import FormPreview from "./preview/FormPreview"; | ||
import { useField } from "formik"; | ||
import { type RJSFSchema } from "@/components/formBuilder/formBuilderTypes"; | ||
|
@@ -34,6 +35,7 @@ const FormBuilder: React.FC<{ | |
return ( | ||
<div className={styles.root}> | ||
<div className={styles.column} data-testid="editor"> | ||
<FormIntroFields name={name} /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These fields are only used once or twice, I extracted this component from |
||
<FormEditor | ||
name={name} | ||
activeField={activeField} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
import React from "react"; | ||
import { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These two components can't be made strict, see automated comment: I think I made no changes to |
||
type SelectStringOption, | ||
type SetActiveField, | ||
} from "@/components/formBuilder/formBuilderTypes"; | ||
import FieldEditor from "./FieldEditor"; | ||
import { | ||
moveStringInArray, | ||
getNormalizedUiOrder, | ||
} from "@/components/formBuilder/formBuilderHelpers"; | ||
import { type Schema } from "@/types/schemaTypes"; | ||
import FieldTemplate from "@/components/form/FieldTemplate"; | ||
import LayoutWidget from "@/components/LayoutWidget"; | ||
import { findLast } from "lodash"; | ||
|
||
export const ActiveField: React.FC<{ | ||
name: string; | ||
activeField: string; | ||
setActiveField: SetActiveField; | ||
fieldTypes: SelectStringOption[]; | ||
schema: Schema; | ||
uiOrder: string[]; | ||
propertyKeys: string[]; | ||
setUiOrder: (uiOrder: string[]) => Promise<void | string[]>; | ||
}> = ({ | ||
name, | ||
activeField, | ||
setActiveField, | ||
fieldTypes, | ||
schema, | ||
uiOrder, | ||
propertyKeys, | ||
setUiOrder, | ||
}) => { | ||
const moveProperty = async (direction: "up" | "down") => { | ||
const nextUiOrder = moveStringInArray( | ||
getNormalizedUiOrder(propertyKeys, uiOrder), | ||
activeField, | ||
direction, | ||
); | ||
await setUiOrder(nextUiOrder); | ||
}; | ||
|
||
// The uiOrder field may not be initialized yet | ||
const order = uiOrder ?? ["*"]; | ||
const canMoveUp = | ||
order.length > 2 | ||
? order[0] !== activeField | ||
: propertyKeys[0] !== activeField; | ||
const canMoveDown = | ||
order.length === propertyKeys.length + 1 | ||
? order.at(-2) !== activeField | ||
: Array.isArray(order) && | ||
findLast(propertyKeys, (key) => !order.includes(key)) !== activeField; | ||
|
||
return ( | ||
<> | ||
<h6>Current Field</h6> | ||
|
||
{Boolean(schema?.properties?.[activeField]) && ( | ||
<FieldEditor | ||
name={name} | ||
propertyName={activeField} | ||
setActiveField={setActiveField} | ||
fieldTypes={fieldTypes} | ||
/> | ||
)} | ||
|
||
{(canMoveUp || canMoveDown) && ( | ||
<FieldTemplate | ||
name="layoutButtons" | ||
label="Field Order" | ||
as={LayoutWidget} | ||
canMoveUp={canMoveUp} | ||
moveUp={async () => { | ||
await moveProperty("up"); | ||
}} | ||
canMoveDown={canMoveDown} | ||
moveDown={async () => { | ||
await moveProperty("down"); | ||
}} | ||
/> | ||
)} | ||
</> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: not a fan of using an
h6
.<p><strong>...</strong><p>
might make more sense here?In general, we shouldn't be skipping heading types. If we want it to be a heading, then we should use the appropriate level and re-style.
Very far from a priority.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think skipping heading levels is not an issue, especially since we don't really use them consistently. However since it's a section header, any heading tag is still preferable to p>strong IMO