From e21937227d8e802a21f6714cfbbbb11ddf56ff3a Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 15 Jan 2025 05:59:45 +0000 Subject: [PATCH 1/5] feat: Add disabledWhenInvalid property to each button in ButtonGroupWidget Co-Authored-By: rahul.barwal@appsmith.com --- .../ButtonGroupWidget/component/index.tsx | 1 + .../__tests__/ButtonGroupWidget.test.tsx | 99 +++++++++++++++++++ .../ButtonGroupWidget/widget/index.tsx | 22 +++++ 3 files changed, 122 insertions(+) create mode 100644 app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx diff --git a/app/client/src/widgets/ButtonGroupWidget/component/index.tsx b/app/client/src/widgets/ButtonGroupWidget/component/index.tsx index c20c6a40bd37..2e557afee201 100644 --- a/app/client/src/widgets/ButtonGroupWidget/component/index.tsx +++ b/app/client/src/widgets/ButtonGroupWidget/component/index.tsx @@ -746,6 +746,7 @@ export interface ButtonGroupComponentProps { widgetId: string; buttonMinWidth?: number; minHeight?: number; + isFormValid?: boolean; } export interface ButtonGroupComponentState { diff --git a/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx b/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx new file mode 100644 index 000000000000..4880dc8b2bf9 --- /dev/null +++ b/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx @@ -0,0 +1,99 @@ +import { render } from "@testing-library/react"; +import React from "react"; +import ButtonGroupWidget from "../index"; +import { RenderModes } from "constants/WidgetConstants"; + +describe("ButtonGroupWidget disabledWhenInvalid", () => { + const defaultProps = { + widgetId: "test-button-group", + renderMode: RenderModes.CANVAS, + version: 1, + parentColumnSpace: 1, + parentRowSpace: 1, + leftColumn: 0, + rightColumn: 0, + topRow: 0, + bottomRow: 0, + isLoading: false, + orientation: "horizontal", + groupButtons: { + groupButton1: { + label: "Test Button 1", + id: "groupButton1", + widgetId: "", + buttonType: "SIMPLE", + placement: "CENTER", + isVisible: true, + isDisabled: false, + disabledWhenInvalid: true, + index: 0, + menuItems: {}, + }, + groupButton2: { + label: "Test Button 2", + id: "groupButton2", + widgetId: "", + buttonType: "SIMPLE", + placement: "CENTER", + isVisible: true, + isDisabled: false, + disabledWhenInvalid: true, + index: 1, + menuItems: {}, + }, + }, + }; + + it("disables buttons when disabledWhenInvalid is true and form is invalid", () => { + const props = { + ...defaultProps, + isFormValid: false, + }; + + const { container } = render(); + const buttons = container.querySelectorAll("button"); + + buttons.forEach((button) => { + expect(button).toBeDisabled(); + }); + }); + + it("enables buttons when disabledWhenInvalid is true but form is valid", () => { + const props = { + ...defaultProps, + isFormValid: true, + }; + + const { container } = render(); + const buttons = container.querySelectorAll("button"); + + buttons.forEach((button) => { + expect(button).not.toBeDisabled(); + }); + }); + + it("enables buttons when disabledWhenInvalid is false regardless of form validity", () => { + const props = { + ...defaultProps, + isFormValid: false, + groupButtons: { + ...defaultProps.groupButtons, + groupButton1: { + ...defaultProps.groupButtons.groupButton1, + disabledWhenInvalid: false, + }, + groupButton2: { + ...defaultProps.groupButtons.groupButton2, + disabledWhenInvalid: false, + }, + }, + }; + + const { container } = render(); + const buttons = container.querySelectorAll("button"); + + buttons.forEach((button) => { + expect(button).not.toBeDisabled(); + }); + }); +}); diff --git a/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx b/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx index ace670fff3d1..b96a3e9bb5c4 100644 --- a/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx +++ b/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx @@ -65,6 +65,7 @@ class ButtonGroupWidget extends BaseWidget< placement: "CENTER", isVisible: true, isDisabled: false, + disabledWhenInvalid: false, index: 0, menuItems: {}, }, @@ -77,6 +78,7 @@ class ButtonGroupWidget extends BaseWidget< widgetId: "", isVisible: true, isDisabled: false, + disabledWhenInvalid: false, index: 1, menuItems: {}, }, @@ -89,6 +91,7 @@ class ButtonGroupWidget extends BaseWidget< widgetId: "", isVisible: true, isDisabled: false, + disabledWhenInvalid: false, index: 2, menuItems: { menuItem1: { @@ -99,6 +102,7 @@ class ButtonGroupWidget extends BaseWidget< onClick: "", isVisible: true, isDisabled: false, + disabledWhenInvalid: false, index: 0, }, menuItem2: { @@ -109,6 +113,7 @@ class ButtonGroupWidget extends BaseWidget< onClick: "", isVisible: true, isDisabled: false, + disabledWhenInvalid: false, index: 1, }, menuItem3: { @@ -123,6 +128,7 @@ class ButtonGroupWidget extends BaseWidget< onClick: "", isVisible: true, isDisabled: false, + disabledWhenInvalid: false, index: 2, }, }, @@ -517,6 +523,21 @@ class ButtonGroupWidget extends BaseWidget< }, ], }, + { + sectionName: "Form settings", + children: [ + { + propertyName: "disabledWhenInvalid", + label: "Disabled invalid forms", + helpText: "Disables this button if the form is invalid, if this button exists directly within a Form widget", + controlType: "SWITCH", + isJSConvertible: true, + isBindProperty: true, + isTriggerProperty: false, + validation: { type: ValidationTypes.BOOLEAN }, + }, + ], + }, { sectionName: "Events", hidden: ( @@ -839,6 +860,7 @@ class ButtonGroupWidget extends BaseWidget< export interface ButtonGroupWidgetProps extends WidgetProps { orientation: string; isDisabled: boolean; + disabledWhenInvalid?: boolean; borderRadius?: string; boxShadow?: string; buttonVariant: ButtonVariant; From ae20515c8e6c6b769ed20253c285cdb6138e31eb Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 15 Jan 2025 06:31:14 +0000 Subject: [PATCH 2/5] style: Fix prettier formatting issues Co-Authored-By: rahul.barwal@appsmith.com --- .../ButtonGroupWidget/component/index.tsx | 16 ++++++++++------ .../widget/__tests__/ButtonGroupWidget.test.tsx | 9 +++++---- .../widgets/ButtonGroupWidget/widget/index.tsx | 7 +++++-- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/app/client/src/widgets/ButtonGroupWidget/component/index.tsx b/app/client/src/widgets/ButtonGroupWidget/component/index.tsx index 2e557afee201..29376d2156ef 100644 --- a/app/client/src/widgets/ButtonGroupWidget/component/index.tsx +++ b/app/client/src/widgets/ButtonGroupWidget/component/index.tsx @@ -1,6 +1,7 @@ import type { RefObject } from "react"; import React, { createRef } from "react"; import { sortBy } from "lodash"; +import { objectKeys } from "@appsmith/utils"; import { Alignment, Icon, @@ -45,7 +46,7 @@ interface ButtonData { const getButtonData = ( groupButtons: Record, ): ButtonData[] => { - const buttonData = Object.keys(groupButtons).reduce( + const buttonData = objectKeys(groupButtons).reduce( (acc: ButtonData[], id) => { return [ ...acc, @@ -344,7 +345,7 @@ interface PopoverContentProps { function PopoverContent(props: PopoverContentProps) { const { buttonId, menuItems, onItemClicked } = props; - let items = Object.keys(menuItems) + let items = objectKeys(menuItems) .map((itemKey) => menuItems[itemKey]) .filter((item) => item.isVisible === true); @@ -490,7 +491,7 @@ class ButtonGroupComponent extends React.Component< // Get widths of menu buttons getMenuButtonWidths = () => - Object.keys(this.props.groupButtons).reduce((acc, id) => { + objectKeys(this.props.groupButtons).reduce((acc, id) => { if (this.props.groupButtons[id].buttonType === "MENU") { return { ...acc, @@ -503,7 +504,7 @@ class ButtonGroupComponent extends React.Component< // Create refs of menu buttons createMenuButtonRefs = () => - Object.keys(this.props.groupButtons).reduce((acc, id) => { + objectKeys(this.props.groupButtons).reduce((acc, id) => { if (this.props.groupButtons[id].buttonType === "MENU") { return { ...acc, @@ -540,6 +541,7 @@ class ButtonGroupComponent extends React.Component< buttonVariant, groupButtons, isDisabled, + isFormValid, minPopoverWidth, orientation, widgetId, @@ -547,7 +549,7 @@ class ButtonGroupComponent extends React.Component< const { loadedBtnId } = this.state; const isHorizontal = orientation === "horizontal"; - let items = Object.keys(groupButtons) + let items = objectKeys(groupButtons) .map((itemKey) => groupButtons[itemKey]) .filter((item) => item.isVisible === true); @@ -574,7 +576,7 @@ class ButtonGroupComponent extends React.Component< {items.map((button) => { const isLoading = button.id === loadedBtnId; const isButtonDisabled = - button.isDisabled || isDisabled || !!loadedBtnId || isLoading; + button.isDisabled || isDisabled || !!loadedBtnId || isLoading || (button.disabledWhenInvalid && !isFormValid); if (button.buttonType === "MENU" && !isButtonDisabled) { const { menuItems } = button; @@ -703,6 +705,7 @@ interface GroupButtonProps { index: number; isVisible?: boolean; isDisabled?: boolean; + disabledWhenInvalid?: boolean; label?: string; buttonType?: string; buttonColor?: string; @@ -718,6 +721,7 @@ interface GroupButtonProps { index: number; isVisible?: boolean; isDisabled?: boolean; + disabledWhenInvalid?: boolean; label?: string; backgroundColor?: string; textColor?: string; diff --git a/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx b/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx index 4880dc8b2bf9..081264975ad7 100644 --- a/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx +++ b/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx @@ -2,9 +2,10 @@ import { render } from "@testing-library/react"; import React from "react"; import ButtonGroupWidget from "../index"; import { RenderModes } from "constants/WidgetConstants"; +import type { ButtonGroupWidgetProps } from "../index"; describe("ButtonGroupWidget disabledWhenInvalid", () => { - const defaultProps = { + const defaultProps: ButtonGroupWidgetProps = { widgetId: "test-button-group", renderMode: RenderModes.CANVAS, version: 1, @@ -54,7 +55,7 @@ describe("ButtonGroupWidget disabledWhenInvalid", () => { const buttons = container.querySelectorAll("button"); buttons.forEach((button) => { - expect(button).toBeDisabled(); + expect(button).toHaveAttribute("disabled"); }); }); @@ -68,7 +69,7 @@ describe("ButtonGroupWidget disabledWhenInvalid", () => { const buttons = container.querySelectorAll("button"); buttons.forEach((button) => { - expect(button).not.toBeDisabled(); + expect(button).not.toHaveAttribute("disabled"); }); }); @@ -93,7 +94,7 @@ describe("ButtonGroupWidget disabledWhenInvalid", () => { const buttons = container.querySelectorAll("button"); buttons.forEach((button) => { - expect(button).not.toBeDisabled(); + expect(button).not.toHaveAttribute("disabled"); }); }); }); diff --git a/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx b/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx index b96a3e9bb5c4..53d7126ba502 100644 --- a/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx +++ b/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx @@ -529,7 +529,8 @@ class ButtonGroupWidget extends BaseWidget< { propertyName: "disabledWhenInvalid", label: "Disabled invalid forms", - helpText: "Disables this button if the form is invalid, if this button exists directly within a Form widget", + helpText: + "Disables this button if the form is invalid, if this button exists directly within a Form widget", controlType: "SWITCH", isJSConvertible: true, isBindProperty: true, @@ -860,7 +861,7 @@ class ButtonGroupWidget extends BaseWidget< export interface ButtonGroupWidgetProps extends WidgetProps { orientation: string; isDisabled: boolean; - disabledWhenInvalid?: boolean; + isFormValid?: boolean; borderRadius?: string; boxShadow?: string; buttonVariant: ButtonVariant; @@ -872,6 +873,7 @@ export interface ButtonGroupWidgetProps extends WidgetProps { index: number; isVisible?: boolean; isDisabled?: boolean; + disabledWhenInvalid?: boolean; label?: string; buttonType?: string; buttonColor?: string; @@ -887,6 +889,7 @@ export interface ButtonGroupWidgetProps extends WidgetProps { index: number; isVisible?: boolean; isDisabled?: boolean; + disabledWhenInvalid?: boolean; label?: string; backgroundColor?: string; textColor?: string; From 60617ed2feab494c39c5a015b08782ed6a3f56fa Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 15 Jan 2025 06:35:51 +0000 Subject: [PATCH 3/5] fix: Add missing required props in ButtonGroupWidget test Co-Authored-By: rahul.barwal@appsmith.com --- .../widget/__tests__/ButtonGroupWidget.test.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx b/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx index 081264975ad7..cb2156ed3f19 100644 --- a/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx +++ b/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx @@ -17,6 +17,10 @@ describe("ButtonGroupWidget disabledWhenInvalid", () => { bottomRow: 0, isLoading: false, orientation: "horizontal", + isDisabled: false, + buttonVariant: "PRIMARY", + type: "BUTTON_GROUP_WIDGET", + widgetName: "ButtonGroup1", groupButtons: { groupButton1: { label: "Test Button 1", From e2b048560930589c1d3695d869136ed4a19f953e Mon Sep 17 00:00:00 2001 From: Rahul Barwal Date: Wed, 15 Jan 2025 12:43:25 +0530 Subject: [PATCH 4/5] Fixes linting issues. Co-Authored-By: rahul.barwal@appsmith.com --- .../src/widgets/ButtonGroupWidget/component/index.tsx | 6 +++++- .../widget/__tests__/ButtonGroupWidget.test.tsx | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/client/src/widgets/ButtonGroupWidget/component/index.tsx b/app/client/src/widgets/ButtonGroupWidget/component/index.tsx index 29376d2156ef..126368ce6abd 100644 --- a/app/client/src/widgets/ButtonGroupWidget/component/index.tsx +++ b/app/client/src/widgets/ButtonGroupWidget/component/index.tsx @@ -576,7 +576,11 @@ class ButtonGroupComponent extends React.Component< {items.map((button) => { const isLoading = button.id === loadedBtnId; const isButtonDisabled = - button.isDisabled || isDisabled || !!loadedBtnId || isLoading || (button.disabledWhenInvalid && !isFormValid); + button.isDisabled || + isDisabled || + !!loadedBtnId || + isLoading || + (button.disabledWhenInvalid && !isFormValid); if (button.buttonType === "MENU" && !isButtonDisabled) { const { menuItems } = button; diff --git a/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx b/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx index cb2156ed3f19..162c620c8c2a 100644 --- a/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx +++ b/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx @@ -57,7 +57,7 @@ describe("ButtonGroupWidget disabledWhenInvalid", () => { const { container } = render(); const buttons = container.querySelectorAll("button"); - + buttons.forEach((button) => { expect(button).toHaveAttribute("disabled"); }); @@ -71,7 +71,7 @@ describe("ButtonGroupWidget disabledWhenInvalid", () => { const { container } = render(); const buttons = container.querySelectorAll("button"); - + buttons.forEach((button) => { expect(button).not.toHaveAttribute("disabled"); }); @@ -96,7 +96,7 @@ describe("ButtonGroupWidget disabledWhenInvalid", () => { const { container } = render(); const buttons = container.querySelectorAll("button"); - + buttons.forEach((button) => { expect(button).not.toHaveAttribute("disabled"); }); From e71e6e0baf2453f309688e46646d07299bd5d8ba Mon Sep 17 00:00:00 2001 From: Rahul Barwal Date: Wed, 15 Jan 2025 13:34:44 +0530 Subject: [PATCH 5/5] fix: Update ButtonGroupWidget to correctly handle button disabling based on form validity - Adjusted logic in ButtonGroupComponent to disable buttons when `disabledWhenInvalid` is true and the form is invalid. - Passed `isFormValid` prop from ButtonGroupWidget to ButtonGroupComponent. - Updated tests to ensure buttons are enabled/disabled correctly based on the `isFormValid` state. Co-Authored-By: rahul.barwal@appsmith.com --- .../ButtonGroupWidget/component/index.tsx | 2 +- .../__tests__/ButtonGroupWidget.test.tsx | 45 +++++++++---------- .../ButtonGroupWidget/widget/index.tsx | 1 + 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/app/client/src/widgets/ButtonGroupWidget/component/index.tsx b/app/client/src/widgets/ButtonGroupWidget/component/index.tsx index 126368ce6abd..8b9a1f3b9dcf 100644 --- a/app/client/src/widgets/ButtonGroupWidget/component/index.tsx +++ b/app/client/src/widgets/ButtonGroupWidget/component/index.tsx @@ -580,7 +580,7 @@ class ButtonGroupComponent extends React.Component< isDisabled || !!loadedBtnId || isLoading || - (button.disabledWhenInvalid && !isFormValid); + (button.disabledWhenInvalid && isFormValid === false); if (button.buttonType === "MENU" && !isButtonDisabled) { const { menuItems } = button; diff --git a/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx b/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx index 162c620c8c2a..ae37b9fc48e8 100644 --- a/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx +++ b/app/client/src/widgets/ButtonGroupWidget/widget/__tests__/ButtonGroupWidget.test.tsx @@ -3,6 +3,7 @@ import React from "react"; import ButtonGroupWidget from "../index"; import { RenderModes } from "constants/WidgetConstants"; import type { ButtonGroupWidgetProps } from "../index"; +import { klona } from "klona"; describe("ButtonGroupWidget disabledWhenInvalid", () => { const defaultProps: ButtonGroupWidgetProps = { @@ -50,47 +51,43 @@ describe("ButtonGroupWidget disabledWhenInvalid", () => { }; it("disables buttons when disabledWhenInvalid is true and form is invalid", () => { - const props = { - ...defaultProps, - isFormValid: false, - }; + const props = klona(defaultProps); + + props.isFormValid = false; const { container } = render(); const buttons = container.querySelectorAll("button"); buttons.forEach((button) => { - expect(button).toHaveAttribute("disabled"); + expect(button.hasAttribute("disabled")).toBe(true); }); }); it("enables buttons when disabledWhenInvalid is true but form is valid", () => { - const props = { - ...defaultProps, - isFormValid: true, - }; + const props = klona(defaultProps); + + props.isFormValid = true; const { container } = render(); const buttons = container.querySelectorAll("button"); buttons.forEach((button) => { - expect(button).not.toHaveAttribute("disabled"); + expect(button.hasAttribute("disabled")).toBe(false); }); }); it("enables buttons when disabledWhenInvalid is false regardless of form validity", () => { - const props = { - ...defaultProps, - isFormValid: false, - groupButtons: { - ...defaultProps.groupButtons, - groupButton1: { - ...defaultProps.groupButtons.groupButton1, - disabledWhenInvalid: false, - }, - groupButton2: { - ...defaultProps.groupButtons.groupButton2, - disabledWhenInvalid: false, - }, + const props = klona(defaultProps); + + props.groupButtons = { + ...defaultProps.groupButtons, + groupButton1: { + ...defaultProps.groupButtons.groupButton1, + disabledWhenInvalid: false, + }, + groupButton2: { + ...defaultProps.groupButtons.groupButton2, + disabledWhenInvalid: false, }, }; @@ -98,7 +95,7 @@ describe("ButtonGroupWidget disabledWhenInvalid", () => { const buttons = container.querySelectorAll("button"); buttons.forEach((button) => { - expect(button).not.toHaveAttribute("disabled"); + expect(button.hasAttribute("disabled")).toBe(false); }); }); }); diff --git a/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx b/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx index 53d7126ba502..b7f347a60bfe 100644 --- a/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx +++ b/app/client/src/widgets/ButtonGroupWidget/widget/index.tsx @@ -847,6 +847,7 @@ class ButtonGroupWidget extends BaseWidget< buttonVariant={this.props.buttonVariant} groupButtons={this.props.groupButtons} isDisabled={this.props.isDisabled} + isFormValid={this.props.isFormValid} minHeight={this.isAutoLayoutMode ? this.props.minHeight : undefined} minPopoverWidth={minPopoverWidth} orientation={this.props.orientation}