Skip to content

Commit

Permalink
Feature: Add enter_to_submit param to st.form (streamlit#9480)
Browse files Browse the repository at this point in the history
Adds a parameter to `st.form` that allows you to disable/enable “Enter to submit” functionality/instruction display.
Also consolidates naming convention from `SubmitOnEnter` to `EnterToSubmit`
  • Loading branch information
mayagbarnes authored Sep 18, 2024
1 parent 002e061 commit 67aaff5
Show file tree
Hide file tree
Showing 28 changed files with 151 additions and 62 deletions.
10 changes: 10 additions & 0 deletions e2e_playwright/st_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,13 @@
)
if submitted_7 or submitted_7b:
st.write("Form submitted")

with st.form("form_8", enter_to_submit=False):
st.write("Inside form 8")
number_input = st.number_input("Form 8 - Number Input", 0, 100, step=1)
submitted_8 = st.form_submit_button(
"Form 8 - Submit",
use_container_width=True,
)
if submitted_8:
st.write("Form submitted")
13 changes: 13 additions & 0 deletions e2e_playwright/st_form_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,19 @@ def test_form_disabled_submit_on_enter(app: Page):
expect(form_7.get_by_test_id("stMarkdown").last).not_to_have_text("Form submitted")


def test_enter_to_submit_false(app: Page):
"""Tests that pressing Enter does not submit form when enter_to_submit=False."""
form_8 = app.get_by_test_id("stForm").nth(7)
number_input = form_8.get_by_test_id("stNumberInput").locator("input")
number_input.fill("42")
expect(form_8.get_by_test_id("InputInstructions")).to_have_text("")

# Try to submit the form by pressing Enter, check not submitted.
number_input.press("Enter")
wait_for_app_run(app)
expect(form_8.get_by_test_id("stMarkdown").last).not_to_have_text("Form submitted")


def test_form_submits_on_click(app: Page):
"""Tests that submit via enabled form submit button works."""
form_6 = app.get_by_test_id("stForm").nth(5)
Expand Down
37 changes: 31 additions & 6 deletions frontend/lib/src/WidgetStateManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ describe("Widget State Manager", () => {
})
})

describe("allowFormSubmitOnEnter", () => {
describe("allowFormEnterToSubmit", () => {
it("returns true for a valid formId with 1st submit button enabled", () => {
// Create form with a submit button
const formId = "mockFormId"
Expand All @@ -681,7 +681,7 @@ describe("Widget State Manager", () => {
// Form should exist & allow submission on Enter
// @ts-expect-error - checking that form exists via internal state
expect(widgetMgr.forms.get(formId)).toBeTruthy()
expect(widgetMgr.allowFormSubmitOnEnter(formId)).toBe(true)
expect(widgetMgr.allowFormEnterToSubmit(formId)).toBe(true)
})

it("returns false for an invalid formId", () => {
Expand All @@ -706,7 +706,7 @@ describe("Widget State Manager", () => {

// @ts-expect-error - Other form should NOT exist & should not allow submit on Enter
expect(widgetMgr.forms.get("INVALID_FORM_ID")).toBeFalsy()
expect(widgetMgr.allowFormSubmitOnEnter("INVALID_FORM_ID")).toBe(false)
expect(widgetMgr.allowFormEnterToSubmit("INVALID_FORM_ID")).toBe(false)
})

it("returns false for a valid formId with no submit buttons", () => {
Expand All @@ -724,7 +724,7 @@ describe("Widget State Manager", () => {

// @ts-expect-error - Created form should exist, but no allow submit on Enter
expect(widgetMgr.forms.get(formId)).toBeTruthy()
expect(widgetMgr.allowFormSubmitOnEnter(formId)).toBe(false)
expect(widgetMgr.allowFormEnterToSubmit(formId)).toBe(false)
})

it("returns false if the 1st submit button disabled", () => {
Expand All @@ -746,7 +746,7 @@ describe("Widget State Manager", () => {

// @ts-expect-error - Created form should exist, but no allow submit on Enter
expect(widgetMgr.forms.get(formId)).toBeTruthy()
expect(widgetMgr.allowFormSubmitOnEnter(formId)).toBe(false)
expect(widgetMgr.allowFormEnterToSubmit(formId)).toBe(false)
})

it("returns true if the 1st submit button enabled, others disabled", () => {
Expand All @@ -772,7 +772,32 @@ describe("Widget State Manager", () => {

// @ts-expect-error - Created form should exist and allow submit on Enter
expect(widgetMgr.forms.get(formId)).toBeTruthy()
expect(widgetMgr.allowFormSubmitOnEnter(formId)).toBe(true)
expect(widgetMgr.allowFormEnterToSubmit(formId)).toBe(true)
})

it("returns false if form created with enter_to_submit=False", () => {
// Create form with a submit button
const formId = "mockFormId"

// Create form with enter_to_submit=False
widgetMgr.setFormSubmitBehaviors(formId, false, false)

widgetMgr.addSubmitButton(
formId,
new ButtonProto({ id: "submitButton" })
)
widgetMgr.setStringValue(
{ id: "widget1", formId },
"foo",
{
fromUi: true,
},
undefined
)

// @ts-expect-error - Created form should exist, but no allow submit on Enter
expect(widgetMgr.forms.get(formId)).toBeTruthy()
expect(widgetMgr.allowFormEnterToSubmit(formId)).toBe(false)
})
})

Expand Down
34 changes: 25 additions & 9 deletions frontend/lib/src/WidgetStateManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ class FormState {
/** True if the form was created with the clear_on_submit flag. */
public clearOnSubmit = false

/** True if the form was created with the enter_to_submit flag. */
public enterToSubmit = true

/** Signal emitted when the form is cleared. */
public readonly formCleared = new Signal()

Expand Down Expand Up @@ -209,11 +212,17 @@ export class WidgetStateManager {
}

/**
* Register a Form, and assign its clearOnSubmit value.
* Register a Form, and assign its clearOnSubmit & enterToSubmit values.
* The `Form` element calls this when it's first mounted.
*/
public setFormClearOnSubmit(formId: string, clearOnSubmit: boolean): void {
this.getOrCreateFormState(formId).clearOnSubmit = clearOnSubmit
public setFormSubmitBehaviors(
formId: string,
clearOnSubmit: boolean,
enterToSubmit = true
): void {
const form = this.getOrCreateFormState(formId)
form.clearOnSubmit = clearOnSubmit
form.enterToSubmit = enterToSubmit
}

/**
Expand Down Expand Up @@ -680,16 +689,23 @@ export class WidgetStateManager {
/**
* Helper function to determine whether a form allows enter to submit
* for input elements (st.number_input, st.text_input, etc.)
* Must be in a form & have 1st submit button enabled to allow
* If in form, checks form's enterToSubmit paramf first, otherwise default
* behavior: Must have 1st submit button enabled to allow
*/
public allowFormSubmitOnEnter(formId: string): boolean {
public allowFormEnterToSubmit(formId: string): boolean {
// Don't allow if not in form
if (!isValidFormId(formId)) return false

// Check if user-set enterToSubmit param is false (in FormState)
const form = this.forms.get(formId)
if (form && !form.enterToSubmit) return false

// Otherwise, use default behavior
const submitButtons = this.formsData.submitButtons.get(formId)
const firstSubmitButton = submitButtons?.[0]

// If no submit buttons for the formId, either not in form or invalid form
if (!firstSubmitButton) {
return false
}
// If no submit buttons for the formId, invalid form
if (!firstSubmitButton) return false

// Allow form submit on enter as long as 1st submit button is not disabled
return !firstSubmitButton.disabled
Expand Down
3 changes: 2 additions & 1 deletion frontend/lib/src/components/core/Block/Block.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ const BlockNodeRenderer = (props: BlockPropsWithWidth): ReactElement => {
}

if (node.deltaBlock.type === "form") {
const { formId, clearOnSubmit, border } = node.deltaBlock
const { formId, clearOnSubmit, enterToSubmit, border } = node.deltaBlock
.form as BlockProto.Form
const submitButtons = props.formsData.submitButtons.get(formId)
const hasSubmitButton =
Expand All @@ -145,6 +145,7 @@ const BlockNodeRenderer = (props: BlockPropsWithWidth): ReactElement => {
<Form
formId={formId}
clearOnSubmit={clearOnSubmit}
enterToSubmit={enterToSubmit}
width={props.width}
hasSubmitButton={hasSubmitButton}
scriptRunState={props.scriptRunState}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,10 @@ describe("InputInstructions", () => {
)
})

it("should not show enter instructions if allowSubmitOnEnter is false", () => {
it("should not show enter instructions if allowEnterToSubmit is false", () => {
const props = getProps({
inForm: true,
allowSubmitOnEnter: false,
allowEnterToSubmit: false,
})
render(<InputInstructions {...props} />)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export interface Props {
maxLength?: number
className?: string
type?: "multiline" | "single" | "chat"
allowSubmitOnEnter?: boolean
allowEnterToSubmit?: boolean
}

const InputInstructions = ({
Expand All @@ -38,7 +38,7 @@ const InputInstructions = ({
maxLength,
className,
type = "single",
allowSubmitOnEnter = true,
allowEnterToSubmit = true,
}: Props): ReactElement => {
const messages: ReactElement[] = []
const addMessage = (text: string, shouldBlink = false): void => {
Expand All @@ -53,8 +53,8 @@ const InputInstructions = ({
)
}

// Show enter instruction if not a form or form allows submit on Enter
if (dirty && allowSubmitOnEnter) {
// Show enter instruction if not a form or form allows Enter to submit
if (dirty && allowEnterToSubmit) {
const toSubmitFormOrApplyText = inForm ? "submit form" : "apply"
if (type === "multiline") {
const commandKey = isFromMac() ? "⌘" : "Ctrl"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ describe("ButtonGroup widget", () => {
formId: "form",
clickMode: ButtonGroupProto.ClickMode.MULTI_SELECT,
})
props.widgetMgr.setFormClearOnSubmit("form", true)
props.widgetMgr.setFormSubmitBehaviors("form", true)

jest.spyOn(props.widgetMgr, "setIntArrayValue")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ describe("Checkbox widget", () => {
it("resets its value when form is cleared", async () => {
// Create a widget in a clearOnSubmit form
const props = getProps({ formId: "form" })
props.widgetMgr.setFormClearOnSubmit("form", true)
props.widgetMgr.setFormSubmitBehaviors("form", true)

jest.spyOn(props.widgetMgr, "setBoolValue")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ describe("ColorPicker widget", () => {
// Create a widget in a clearOnSubmit form
const props = getProps({ formId: "form" })
jest.spyOn(props.widgetMgr, "setStringValue")
props.widgetMgr.setFormClearOnSubmit("form", true)
props.widgetMgr.setFormSubmitBehaviors("form", true)

render(<ColorPicker {...props} />)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ describe("DateInput widget", () => {
it("resets its value when form is cleared", () => {
// Create a widget in a clearOnSubmit form
const props = getProps({ formId: "form" })
props.widgetMgr.setFormClearOnSubmit("form", true)
props.widgetMgr.setFormSubmitBehaviors("form", true)

jest.spyOn(props.widgetMgr, "setStringArrayValue")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ describe("FileUploader widget tests", () => {
// Create a widget in a clearOnSubmit form
const props = getProps({ formId: "form" })
jest.spyOn(props.widgetMgr, "setFileUploaderStateValue")
props.widgetMgr.setFormClearOnSubmit("form", true)
props.widgetMgr.setFormSubmitBehaviors("form", true)

jest.spyOn(props.widgetMgr, "setIntValue")

Expand Down
1 change: 1 addition & 0 deletions frontend/lib/src/components/widgets/Form/Form.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ describe("Form", () => {
hasSubmitButton: false,
scriptRunState: ScriptRunState.RUNNING,
clearOnSubmit: false,
enterToSubmit: true,
widgetMgr: new WidgetStateManager({
sendRerunBackMsg: jest.fn(),
formsDataChanged: jest.fn(),
Expand Down
9 changes: 5 additions & 4 deletions frontend/lib/src/components/widgets/Form/Form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { StyledErrorContainer, StyledForm } from "./styled-components"
export interface Props {
formId: string
clearOnSubmit: boolean
enterToSubmit: boolean
width: number
hasSubmitButton: boolean
scriptRunState: ScriptRunState
Expand All @@ -51,14 +52,14 @@ export function Form(props: Props): ReactElement {
width,
scriptRunState,
clearOnSubmit,
enterToSubmit,
border,
} = props

// Tell WidgetStateManager if this form is `clearOnSubmit` so that it can
// do the right thing when the form is submitted.
// Tell WidgetStateManager if this form is `clearOnSubmit` and `enterToSubmit`
useEffect(() => {
widgetMgr.setFormClearOnSubmit(formId, clearOnSubmit)
}, [widgetMgr, formId, clearOnSubmit])
widgetMgr.setFormSubmitBehaviors(formId, clearOnSubmit, enterToSubmit)
}, [widgetMgr, formId, clearOnSubmit, enterToSubmit])

// Determine if we need to show the "missing submit button" warning.
// If we have a submit button, we don't show the warning, of course.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ describe("Multiselect widget", () => {
it("resets its value when form is cleared", () => {
// Create a widget in a clearOnSubmit form
const props = getProps({ formId: "form" })
props.widgetMgr.setFormClearOnSubmit("form", true)
props.widgetMgr.setFormSubmitBehaviors("form", true)

jest.spyOn(props.widgetMgr, "setIntArrayValue")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ describe("NumberInput widget", () => {
it("resets its value when form is cleared", () => {
// Create a widget in a clearOnSubmit form
const props = getIntProps({ formId: "form", default: 10 })
props.widgetMgr.setFormClearOnSubmit("form", true)
props.widgetMgr.setFormSubmitBehaviors("form", true)

jest.spyOn(props.widgetMgr, "setIntValue")
render(<NumberInput {...props} />)
Expand Down Expand Up @@ -222,7 +222,7 @@ describe("NumberInput widget", () => {
it("shows Input Instructions if in form that allows submit on enter", async () => {
const user = userEvent.setup()
const props = getIntProps({ formId: "form" })
jest.spyOn(props.widgetMgr, "allowFormSubmitOnEnter").mockReturnValue(true)
jest.spyOn(props.widgetMgr, "allowFormEnterToSubmit").mockReturnValue(true)

render(<NumberInput {...props} />)
const numberInput = screen.getByTestId("stNumberInputField")
Expand All @@ -240,7 +240,7 @@ describe("NumberInput widget", () => {
const user = userEvent.setup()
const props = getIntProps({ formId: "form" })
jest
.spyOn(props.widgetMgr, "allowFormSubmitOnEnter")
.spyOn(props.widgetMgr, "allowFormEnterToSubmit")
.mockReturnValue(false)

render(<NumberInput {...props} />)
Expand Down
10 changes: 5 additions & 5 deletions frontend/lib/src/components/widgets/NumberInput/NumberInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ export const NumberInput: React.FC<Props> = ({

const inForm = isInForm({ formId: elementFormId })
// Allows form submission on Enter & displays Enter instructions
const allowFormSubmitOnEnter =
widgetMgr.allowFormSubmitOnEnter(elementFormId)
const allowFormEnterToSubmit =
widgetMgr.allowFormEnterToSubmit(elementFormId)

// update the step if the props change
React.useEffect(() => {
Expand Down Expand Up @@ -380,7 +380,7 @@ export const NumberInput: React.FC<Props> = ({
if (dirty) {
commitValue({ value, source: { fromUi: true } })
}
if (allowFormSubmitOnEnter) {
if (allowFormEnterToSubmit) {
widgetMgr.submitForm(elementFormId, fragmentId)
}
}
Expand All @@ -392,7 +392,7 @@ export const NumberInput: React.FC<Props> = ({
widgetMgr,
elementFormId,
fragmentId,
allowFormSubmitOnEnter,
allowFormEnterToSubmit,
]
)

Expand Down Expand Up @@ -531,7 +531,7 @@ export const NumberInput: React.FC<Props> = ({
dirty={dirty}
value={formattedValue ?? ""}
inForm={inForm}
allowSubmitOnEnter={allowFormSubmitOnEnter || !inForm}
allowEnterToSubmit={allowFormEnterToSubmit || !inForm}
/>
</StyledInstructionsContainer>
)}
Expand Down
2 changes: 1 addition & 1 deletion frontend/lib/src/components/widgets/Radio/Radio.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ describe("Radio widget", () => {
it("resets its value when form is cleared", async () => {
// Create a widget in a clearOnSubmit form
const props = getProps({ formId: "form" })
props.widgetMgr.setFormClearOnSubmit("form", true)
props.widgetMgr.setFormSubmitBehaviors("form", true)

jest.spyOn(props.widgetMgr, "setIntValue")
render(<Radio {...props} />)
Expand Down
Loading

0 comments on commit 67aaff5

Please sign in to comment.