-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: implement disabled functionality in the property pane section and control #39939
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
Changes from all commits
9e6417d
86bd4b0
52dfadf
6132745
19c3620
7a3e6cc
845406a
5a57c17
b1adca0
d8cba00
14db8bb
3536352
2a75ea6
864f329
007a525
5a68737
f79ccfd
d4f7b64
aa2b8fc
2164882
17a09a3
7b5cd76
efa3347
1e656cf
08429fe
285d46d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,6 +53,19 @@ export interface PropertyPaneSectionConfig { | |
| // TODO: Fix this the next time the file is edited | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| hidden?: (props: any, propertyPath: string) => boolean; | ||
| /** | ||
| * @param props - Current widget properties | ||
| * @param propertyPath - Path to the widget property | ||
| * @returns - True if the section should be disabled, false otherwise | ||
| */ | ||
| // TODO: Fix this the next time the file is edited | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
|
Contributor
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. Can we remove this? Let's not use any; you can prefer using
Contributor
Author
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. I have tried widget props and I still get a lot of errors on CI - build failure due to type check. |
||
| shouldDisableSection?: (props: any, propertyPath: string) => boolean; | ||
| /** | ||
| * Help text to show when section is disabled. | ||
| * Appears as a tooltip when hovering over the disabled section. | ||
| */ | ||
| disabledHelpText?: string; | ||
| /** | ||
| * when true, the section will be open by default. | ||
| * Note: Seems like this is not used anywhere. | ||
|
|
@@ -205,6 +218,20 @@ export interface PropertyPaneControlConfig { | |
| // TODO: Fix this the next time the file is edited | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| hidden?: (props: any, propertyPath: string) => boolean; | ||
|
Contributor
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. π οΈ Refactor suggestion Update this property to use more specific typing. Similar to line 53, this property should be updated from - // TODO: Fix this the next time the file is edited
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- hidden?: (props: any, propertyPath: string) => boolean;
+ hidden?: (props: Record<string, unknown>, propertyPath: string) => boolean;
|
||
| /** | ||
| * callback function to determine if the property should be disabled. | ||
|
|
||
| * @param props - Current widget properties | ||
| * @param propertyPath - Path to the widget property | ||
| * @returns - True if the property should be disabled, false otherwise | ||
| */ | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| shouldDisableSection?: (props: any, propertyPath: string) => boolean; | ||
| /** | ||
| * Help text to show when property is disabled. | ||
| * Appears as a tooltip when hovering over the disabled property. | ||
| */ | ||
| disabledHelpText?: string; | ||
| /** | ||
| * If true, the property is hidden. | ||
| * Note: hidden and invisible do the same thing but differently. hidden uses a callback to determine if the property should be hidden. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| import React from "react"; | ||
| import { render } from "@testing-library/react"; | ||
| import { EditorTheme } from "components/editorComponents/CodeEditor/EditorConfig"; | ||
| import type { IPanelProps } from "@blueprintjs/core"; | ||
| import type { WidgetProps } from "widgets/BaseWidget"; | ||
| import PropertyControl from "./PropertyControl"; | ||
| import type { EnhancementFns } from "selectors/widgetEnhancementSelectors"; | ||
|
|
||
| interface MockPropertyControlProps { | ||
| shouldDisableSection?: ( | ||
| widgetProperties: WidgetProps, | ||
| propertyName: string, | ||
| ) => boolean; | ||
| disabledHelpText?: string; | ||
| label: string; | ||
| propertyName: string; | ||
| widgetProperties: WidgetProps; | ||
| } | ||
|
|
||
| const MockPropertyControl = (props: MockPropertyControlProps) => { | ||
| const isDisabled = props.shouldDisableSection | ||
| ? props.shouldDisableSection(props.widgetProperties, props.propertyName) | ||
| : false; | ||
|
|
||
| return ( | ||
| <div | ||
| className={isDisabled ? "cursor-not-allowed opacity-50" : ""} | ||
| data-testid="t--property-control-wrapper" | ||
| > | ||
| <label>{props.label}</label> | ||
| <input | ||
| data-testid="t--property-input" | ||
| disabled={isDisabled} | ||
| role="textbox" | ||
| type="text" | ||
| /> | ||
| {isDisabled && props.disabledHelpText && ( | ||
| <div data-testid="t--disabled-help-text">{props.disabledHelpText}</div> | ||
| )} | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| jest.mock("./PropertyControl", () => (props: MockPropertyControlProps) => ( | ||
| <MockPropertyControl {...props} /> | ||
| )); | ||
|
|
||
| describe("PropertyControl", () => { | ||
| const mockPanel: IPanelProps = { | ||
| closePanel: jest.fn(), | ||
| openPanel: jest.fn(), | ||
| }; | ||
|
|
||
| const defaultProps = { | ||
| controlType: "INPUT_TEXT", | ||
| enhancements: undefined as EnhancementFns | undefined, | ||
| isBindProperty: true, | ||
| isSearchResult: false, | ||
| isTriggerProperty: false, | ||
| label: "Test Label", | ||
| panel: mockPanel, | ||
| propertyName: "testProperty", | ||
| theme: EditorTheme.LIGHT, | ||
| widgetProperties: { | ||
| testProperty: "test value", | ||
| type: "CONTAINER_WIDGET", | ||
| widgetId: "test-widget", | ||
| widgetName: "TestWidget", | ||
| }, | ||
| }; | ||
|
|
||
| it("should render property control normally when not disabled", () => { | ||
| const { getByTestId } = render(<PropertyControl {...defaultProps} />); | ||
|
|
||
| expect( | ||
| (getByTestId("t--property-input") as HTMLInputElement).disabled, | ||
| ).toBe(false); | ||
| expect(getByTestId("t--property-control-wrapper").className).toBe(""); | ||
| }); | ||
|
|
||
| it("should render disabled property control when disabled prop is true", () => { | ||
| const disabledProps = { | ||
| ...defaultProps, | ||
| shouldDisableSection: () => true, | ||
| }; | ||
|
|
||
| const { getByTestId } = render(<PropertyControl {...disabledProps} />); | ||
|
|
||
| const wrapper = getByTestId("t--property-control-wrapper"); | ||
|
|
||
| expect(wrapper.classList.contains("cursor-not-allowed")).toBe(true); | ||
| expect(wrapper.classList.contains("opacity-50")).toBe(true); | ||
| expect( | ||
| (getByTestId("t--property-input") as HTMLInputElement).disabled, | ||
| ).toBe(true); | ||
| }); | ||
|
|
||
| it("should show disabled help text when property is disabled", () => { | ||
| const disabledProps = { | ||
| ...defaultProps, | ||
| shouldDisableSection: () => true, | ||
| disabledHelpText: "This property is disabled because...", | ||
| }; | ||
|
|
||
| const { getByTestId } = render(<PropertyControl {...disabledProps} />); | ||
|
|
||
| expect(getByTestId("t--disabled-help-text")).toBeTruthy(); | ||
| expect(getByTestId("t--disabled-help-text").textContent).toBe( | ||
| "This property is disabled because...", | ||
| ); | ||
| }); | ||
|
|
||
| it("should not show disabled help text when property is not disabled", () => { | ||
| const props = { | ||
| ...defaultProps, | ||
| shouldDisableSection: () => false, | ||
| disabledHelpText: "This property is disabled because...", | ||
| }; | ||
|
|
||
| const { queryByTestId } = render(<PropertyControl {...props} />); | ||
|
|
||
| expect(queryByTestId("t--disabled-help-text")).toBeFalsy(); | ||
| }); | ||
| }); |
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.
Nice work refactoring this!