-
Notifications
You must be signed in to change notification settings - Fork 4.5k
chore: fix menu widget bugs #38226
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
chore: fix menu widget bugs #38226
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| export const anvilConfig = { | ||
| isLargeWidget: false, | ||
| widgetSize: { | ||
| maxWidth: { | ||
| base: "100%", | ||
| "280px": "sizing-70", | ||
| }, | ||
| minWidth: "sizing-14", | ||
| }, | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| export * from "./propertyPaneConfig"; | ||
| export { autocompleteConfig } from "./autocompleteConfig"; | ||
| export { defaultsConfig } from "./defaultsConfig"; | ||
| export { metaConfig } from "./metaConfig"; | ||
| export { anvilConfig } from "./anvilConfig"; | ||
| export { settersConfig } from "./settersConfig"; | ||
| export { methodsConfig } from "./methodsConfig"; | ||
| export { defaultsConfig } from "./defaultsConfig"; | ||
| export { autocompleteConfig } from "./autocompleteConfig"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,81 +1,61 @@ | ||
| import React from "react"; | ||
| import type { SetterConfig } from "entities/AppTheming"; | ||
| import type { WidgetState } from "widgets/BaseWidget"; | ||
| import BaseWidget from "widgets/BaseWidget"; | ||
| import { | ||
| metaConfig, | ||
| defaultsConfig, | ||
| autocompleteConfig, | ||
| propertyPaneContentConfig, | ||
| propertyPaneStyleConfig, | ||
| settersConfig, | ||
| methodsConfig, | ||
| } from "../config"; | ||
| import type { AnvilConfig } from "WidgetProvider/constants"; | ||
| import { Button, MenuTrigger, Menu } from "@appsmith/wds"; | ||
| import React, { type Key } from "react"; | ||
| import { isArray, orderBy } from "lodash"; | ||
| import type { MenuButtonWidgetProps, MenuItem } from "./types"; | ||
| import BaseWidget from "widgets/BaseWidget"; | ||
| import type { WidgetState } from "widgets/BaseWidget"; | ||
| import type { SetterConfig } from "entities/AppTheming"; | ||
| import { | ||
| EventType, | ||
| type ExecuteTriggerPayload, | ||
| } from "constants/AppsmithActionConstants/ActionConstants"; | ||
| import type { AnvilConfig } from "WidgetProvider/constants"; | ||
| import { Button, MenuTrigger, Menu, MenuItem } from "@appsmith/wds"; | ||
|
|
||
| import * as config from "../config"; | ||
| import type { MenuButtonWidgetProps } from "./types"; | ||
|
|
||
| class WDSMenuButtonWidget extends BaseWidget< | ||
| MenuButtonWidgetProps, | ||
| WidgetState | ||
| > { | ||
| constructor(props: MenuButtonWidgetProps) { | ||
| super(props); | ||
|
|
||
| this.state = { | ||
| isLoading: false, | ||
| }; | ||
| } | ||
|
Comment on lines
20
to
22
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 Remove unnecessary constructor The constructor can be removed as it only calls the parent constructor without adding any initialization logic. - constructor(props: MenuButtonWidgetProps) {
- super(props);
- }🧰 Tools🪛 Biome (1.9.4)[error] 20-22: This constructor is unnecessary. Unsafe fix: Remove the unnecessary constructor. (lint/complexity/noUselessConstructor) |
||
|
|
||
| static type = "WDS_MENU_BUTTON_WIDGET"; | ||
|
|
||
| static getConfig() { | ||
| return metaConfig; | ||
| return config.metaConfig; | ||
| } | ||
|
|
||
| static getDefaults() { | ||
| return defaultsConfig; | ||
| return config.defaultsConfig; | ||
| } | ||
|
|
||
| static getAnvilConfig(): AnvilConfig | null { | ||
| return { | ||
| isLargeWidget: false, | ||
| widgetSize: { | ||
| maxWidth: { | ||
| base: "100%", | ||
| "280px": "sizing-70", | ||
| }, | ||
| minWidth: "sizing-14", | ||
| }, | ||
| }; | ||
| return config.anvilConfig; | ||
| } | ||
|
|
||
| static getAutocompleteDefinitions() { | ||
| return autocompleteConfig; | ||
| return config.autocompleteConfig; | ||
| } | ||
|
|
||
| static getPropertyPaneContentConfig() { | ||
| return propertyPaneContentConfig; | ||
| return config.propertyPaneContentConfig; | ||
| } | ||
|
|
||
| static getPropertyPaneStyleConfig() { | ||
| return propertyPaneStyleConfig; | ||
| return config.propertyPaneStyleConfig; | ||
| } | ||
|
|
||
| static getSetterConfig(): SetterConfig { | ||
| return settersConfig; | ||
| return config.settersConfig; | ||
| } | ||
|
|
||
| static getMethods() { | ||
| return methodsConfig; | ||
| return config.methodsConfig; | ||
| } | ||
|
|
||
| menuItemClickHandler = (onClick: string | undefined, index: number) => { | ||
| onMenuItemClick = (onClick: string | undefined, index: number) => { | ||
| if (onClick) { | ||
| const config: ExecuteTriggerPayload = { | ||
| triggerPropertyName: "onClick", | ||
|
|
@@ -85,6 +65,9 @@ class WDSMenuButtonWidget extends BaseWidget< | |
| }, | ||
| }; | ||
|
|
||
| // in case when the menu items source is dynamic, we need to pass the current item to the global context | ||
| // the reason is in onClick, we can access the current item and current index by writing `{{currentItem}}` and `{{currentIndex}}`, | ||
| // so we need to pass the current item to the global context | ||
| if (this.props.menuItemsSource === "dynamic") { | ||
| config.globalContext = { | ||
| currentItem: this.props.sourceData | ||
|
|
@@ -108,15 +91,20 @@ class WDSMenuButtonWidget extends BaseWidget< | |
| .filter((item) => item.isVisible === true); | ||
|
|
||
| return orderBy(visibleItems, ["index"], ["asc"]); | ||
| } else if ( | ||
| } | ||
|
|
||
| if ( | ||
| menuItemsSource === "dynamic" && | ||
| isArray(sourceData) && | ||
| sourceData?.length && | ||
| configureMenuItems?.config | ||
| ) { | ||
| const { config } = configureMenuItems; | ||
|
|
||
| const getValue = (propertyName: keyof MenuItem, index: number) => { | ||
| const getDynamicMenuItemValue = ( | ||
| propertyName: keyof typeof config, | ||
| index: number, | ||
| ) => { | ||
| const value = config[propertyName]; | ||
|
|
||
| if (isArray(value)) { | ||
|
|
@@ -130,15 +118,12 @@ class WDSMenuButtonWidget extends BaseWidget< | |
| .map((item, index) => ({ | ||
| ...item, | ||
| id: index.toString(), | ||
| isVisible: getValue("isVisible", index), | ||
| isDisabled: getValue("isDisabled", index), | ||
| isVisible: getDynamicMenuItemValue("isVisible", index), | ||
| isDisabled: getDynamicMenuItemValue("isDisabled", index), | ||
| index: index, | ||
| widgetId: "", | ||
| label: getValue("label", index), | ||
| label: getDynamicMenuItemValue("label", index), | ||
| onClick: config?.onClick, | ||
| iconAlign: getValue("iconAlign", index), | ||
| iconName: getValue("iconName", index), | ||
| textColor: getValue("textColor", index), | ||
| })) | ||
| .filter((item) => item.isVisible === true); | ||
|
|
||
|
|
@@ -159,7 +144,7 @@ class WDSMenuButtonWidget extends BaseWidget< | |
| triggerButtonVariant, | ||
| } = this.props; | ||
|
|
||
| const visibleItems: MenuItem[] = this.getVisibleItems(); | ||
| const visibleItems = this.getVisibleItems(); | ||
| const disabledKeys = visibleItems | ||
| .filter((item) => item.isDisabled === true) | ||
| .map((item) => item.id); | ||
|
|
@@ -178,21 +163,30 @@ class WDSMenuButtonWidget extends BaseWidget< | |
| </Button> | ||
|
|
||
| <Menu | ||
| disabledKeys={disabledKeys} | ||
| items={visibleItems} | ||
| disabledKeys={disabledKeys as Iterable<Key>} | ||
| onAction={(key) => { | ||
| const clickedItemIndex = visibleItems.findIndex( | ||
| (item) => item.id === key, | ||
| ); | ||
|
|
||
| if (clickedItemIndex > -1) { | ||
| this.menuItemClickHandler( | ||
| this.onMenuItemClick( | ||
| visibleItems[clickedItemIndex]?.onClick, | ||
| clickedItemIndex, | ||
| ); | ||
| } | ||
| }} | ||
| /> | ||
| > | ||
| {visibleItems.map((item) => ( | ||
| <MenuItem | ||
| id={item.id as Key} | ||
| key={item.id as Key} | ||
| textValue={item.label} | ||
| > | ||
| {item.label} | ||
| </MenuItem> | ||
| ))} | ||
KelvinOm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| </Menu> | ||
| </MenuTrigger> | ||
| ); | ||
| } | ||
|
|
||
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.
Why are we doing this again? Could you leave a comment about this?
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.
We are just hiding the option of creating dynamic menu items.
This one.
