From f193d16f72987e32e7c707ccd1dc47b01724b8a7 Mon Sep 17 00:00:00 2001 From: anuraghazra Date: Tue, 17 Dec 2024 02:08:40 +0530 Subject: [PATCH 1/8] chore: add styled props to iconbutton --- .../blade/src/components/Button/IconButton/IconButton.tsx | 3 +++ .../components/Button/IconButton/StyledIconButton.web.tsx | 5 ++++- packages/blade/src/components/Button/IconButton/types.ts | 4 +++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/blade/src/components/Button/IconButton/IconButton.tsx b/packages/blade/src/components/Button/IconButton/IconButton.tsx index 2493d3de7c8..c72b93db183 100644 --- a/packages/blade/src/components/Button/IconButton/IconButton.tsx +++ b/packages/blade/src/components/Button/IconButton/IconButton.tsx @@ -9,6 +9,7 @@ import type { BladeCommonEvents } from '~components/types'; import type { Platform } from '~utils'; import type { SubtleOrIntense } from '~tokens/theme/theme'; import { makeAnalyticsAttribute } from '~utils/makeAnalyticsAttribute'; +import type { StyledPropsBlade } from '~components/Box/styledProps'; type IconButtonProps = { /** @@ -46,6 +47,7 @@ type IconButtonProps = { _tabIndex?: number; } & DataAnalyticsAttribute & BladeCommonEvents & + StyledPropsBlade & Platform.Select<{ web: { onClick: (event: React.MouseEvent) => void; @@ -108,6 +110,7 @@ const _IconButton: React.ForwardRefRenderFunction ); }; diff --git a/packages/blade/src/components/Button/IconButton/StyledIconButton.web.tsx b/packages/blade/src/components/Button/IconButton/StyledIconButton.web.tsx index 32be32289ec..907d1933824 100644 --- a/packages/blade/src/components/Button/IconButton/StyledIconButton.web.tsx +++ b/packages/blade/src/components/Button/IconButton/StyledIconButton.web.tsx @@ -13,6 +13,7 @@ import { getFocusRingStyles } from '~utils/getFocusRingStyles'; import { throwBladeError } from '~utils/logger'; import getIn from '~utils/lodashButBetter/get'; import { makeAnalyticsAttribute } from '~utils/makeAnalyticsAttribute'; +import { useStyledProps } from '~components/Box/styledProps'; type StyledButtonProps = { emphasis: SubtleOrIntense; @@ -23,7 +24,7 @@ type StyledButtonProps = { const StyledButton = styled.button((props) => { const { theme, emphasis } = props; const motionToken = theme.motion; - + const styledPropsCSSObject = useStyledProps(props); const emphasisColor = emphasis === 'intense' ? 'gray' : 'staticWhite'; if (__DEV__) { @@ -74,6 +75,7 @@ const StyledButton = styled.button((props) => { '&:active': { color: theme.colors.interactive.icon[emphasisColor].subtle, }, + ...styledPropsCSSObject, }; }); @@ -121,6 +123,7 @@ const StyledIconButton = React.forwardRef diff --git a/packages/blade/src/components/Button/IconButton/types.ts b/packages/blade/src/components/Button/IconButton/types.ts index 8e386950a33..d4f2aca3262 100644 --- a/packages/blade/src/components/Button/IconButton/types.ts +++ b/packages/blade/src/components/Button/IconButton/types.ts @@ -3,6 +3,7 @@ import type { IconComponent } from '~components/Icons'; import type { DataAnalyticsAttribute, RemoveUndefinedFromUnion, TestID } from '~utils/types'; import type { BladeCommonEvents } from '~components/types'; import type { SubtleOrIntense } from '~tokens/theme/theme'; +import type { StyledPropsBlade } from '~components/Box/styledProps'; export type StyledIconButtonProps = { icon: IconComponent; @@ -15,4 +16,5 @@ export type StyledIconButtonProps = { onClick?: IconButtonProps['onClick']; } & TestID & BladeCommonEvents & - DataAnalyticsAttribute; + DataAnalyticsAttribute & + StyledPropsBlade; From 5caa308d35cf74aea89d210f95a87259a9373136 Mon Sep 17 00:00:00 2001 From: anuraghazra Date: Tue, 17 Dec 2024 02:09:24 +0530 Subject: [PATCH 2/8] chore: button group changes --- .../ButtonGroup/ButtonGroup.web.tsx | 30 +++++-------------- .../ButtonGroup/docs/ButtonGroup.stories.tsx | 5 +++- .../blade/src/components/ButtonGroup/types.ts | 4 ++- .../src/components/Tooltip/Tooltip.web.tsx | 8 ++++- .../src/components/Tooltip/componentIds.ts | 5 ++++ 5 files changed, 26 insertions(+), 26 deletions(-) create mode 100644 packages/blade/src/components/Tooltip/componentIds.ts diff --git a/packages/blade/src/components/ButtonGroup/ButtonGroup.web.tsx b/packages/blade/src/components/ButtonGroup/ButtonGroup.web.tsx index 43aa1322bb5..8e39143b187 100644 --- a/packages/blade/src/components/ButtonGroup/ButtonGroup.web.tsx +++ b/packages/blade/src/components/ButtonGroup/ButtonGroup.web.tsx @@ -1,4 +1,3 @@ -import type { ReactElement } from 'react'; import React from 'react'; import styled from 'styled-components'; import type { ButtonGroupProps } from './types'; @@ -13,10 +12,9 @@ import type { DotNotationToken } from '~utils/lodashButBetter/get'; import getIn from '~utils/lodashButBetter/get'; import { getBackgroundColorToken } from '~components/Button/BaseButton/BaseButton'; import type { Theme } from '~components/BladeProvider'; -import { throwBladeError } from '~utils/logger'; -import { isValidAllowedChildren } from '~utils/isValidAllowedChildren'; import type { BladeElementRef } from '~utils/types'; import { makeAnalyticsAttribute } from '~utils/makeAnalyticsAttribute'; +import { useVerifyAllowedChildren } from '~utils/useVerifyAllowedChildren'; const getDividerColorToken = ({ color, @@ -74,6 +72,12 @@ const _ButtonGroup = ( isFullWidth, }; + useVerifyAllowedChildren({ + allowedComponents: ['Button', 'Dropdown', 'Tooltip', 'Popover'], + componentName: 'ButtonGroup', + children, + }); + return ( {React.Children.map(children, (child, index) => { - if (__DEV__) { - // throw error if child is not a button or dropdown with button trigger - /* eslint-disable no-restricted-properties */ - if ( - !isValidAllowedChildren(child, 'Button') && - !( - isValidAllowedChildren(child, 'Dropdown') && - (child as ReactElement).props.children.some((c: ReactElement) => - isValidAllowedChildren(c, 'DropdownButton'), - ) - ) - ) { - throwBladeError({ - moduleName: 'ButtonGroup', - message: `Only "Button" or "Dropdown" component with Button trigger are allowed as children.`, - }); - } - /* eslint-enable no-restricted-properties */ - } - return ( <> {child} diff --git a/packages/blade/src/components/ButtonGroup/docs/ButtonGroup.stories.tsx b/packages/blade/src/components/ButtonGroup/docs/ButtonGroup.stories.tsx index 518b592790b..bc833391dcc 100644 --- a/packages/blade/src/components/ButtonGroup/docs/ButtonGroup.stories.tsx +++ b/packages/blade/src/components/ButtonGroup/docs/ButtonGroup.stories.tsx @@ -10,6 +10,7 @@ import { getStyledPropsArgTypes } from '~components/Box/BaseBox/storybookArgType import { RefreshIcon, ShareIcon, DownloadIcon, ChevronDownIcon, PlusIcon } from '~components/Icons'; import { Dropdown, DropdownButton, DropdownOverlay } from '~components/Dropdown'; import { ActionList, ActionListItem } from '~components/ActionList'; +import { Tooltip } from '~components/Tooltip'; const Page = (): React.ReactElement => { return ( @@ -76,7 +77,9 @@ const ButtonGroupDropdownTemplate: StoryFn = (args) return ( - + + + diff --git a/packages/blade/src/components/ButtonGroup/types.ts b/packages/blade/src/components/ButtonGroup/types.ts index 156c76b0f76..de6a7a2b91a 100644 --- a/packages/blade/src/components/ButtonGroup/types.ts +++ b/packages/blade/src/components/ButtonGroup/types.ts @@ -1,3 +1,4 @@ +import type { StyledPropsBlade } from '~components/Box/styledProps'; import type { ButtonProps } from '~components/Button/Button'; import type { DataAnalyticsAttribute } from '~utils/types'; @@ -38,7 +39,8 @@ type ButtonGroupProps = { * Test ID for automation */ testID?: string; -} & DataAnalyticsAttribute; +} & DataAnalyticsAttribute & + StyledPropsBlade; type StyledButtonGroupProps = Pick< ButtonGroupProps, diff --git a/packages/blade/src/components/Tooltip/Tooltip.web.tsx b/packages/blade/src/components/Tooltip/Tooltip.web.tsx index c6a5c3916ba..f270164c54a 100644 --- a/packages/blade/src/components/Tooltip/Tooltip.web.tsx +++ b/packages/blade/src/components/Tooltip/Tooltip.web.tsx @@ -20,6 +20,7 @@ import type { TooltipProps } from './types'; import { TooltipContent } from './TooltipContent'; import { ARROW_HEIGHT, ARROW_WIDTH } from './constants'; import { TooltipContext } from './TooltipContext'; +import { componentIds } from './componentIds'; import { useTheme } from '~components/BladeProvider'; import BaseBox from '~components/Box/BaseBox'; import { metaAttribute, MetaConstants } from '~utils/metaAttribute'; @@ -31,8 +32,9 @@ import { PopupArrow } from '~components/PopupArrow'; import { getFloatingPlacementParts } from '~utils/getFloatingPlacementParts'; import { componentZIndices } from '~utils/componentZIndices'; import { makeAnalyticsAttribute } from '~utils/makeAnalyticsAttribute'; +import { assignWithoutSideEffects } from '~utils/assignWithoutSideEffects'; -const Tooltip = ({ +const _Tooltip = ({ title, content, children, @@ -131,4 +133,8 @@ const Tooltip = ({ ); }; +const Tooltip = assignWithoutSideEffects(_Tooltip, { + componentId: componentIds.Tooltip, +}); + export { Tooltip }; diff --git a/packages/blade/src/components/Tooltip/componentIds.ts b/packages/blade/src/components/Tooltip/componentIds.ts new file mode 100644 index 00000000000..2ef2a9202a4 --- /dev/null +++ b/packages/blade/src/components/Tooltip/componentIds.ts @@ -0,0 +1,5 @@ +const componentIds = { + Tooltip: 'Tooltip', +}; + +export { componentIds }; From 92bd8a91582324398fccb37812525587e1196f28 Mon Sep 17 00:00:00 2001 From: anuraghazra Date: Tue, 17 Dec 2024 02:10:04 +0530 Subject: [PATCH 3/8] chore: expose drawer ref --- .../src/components/Drawer/Drawer.web.tsx | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/packages/blade/src/components/Drawer/Drawer.web.tsx b/packages/blade/src/components/Drawer/Drawer.web.tsx index 8a595785972..7c58b62e09e 100644 --- a/packages/blade/src/components/Drawer/Drawer.web.tsx +++ b/packages/blade/src/components/Drawer/Drawer.web.tsx @@ -21,6 +21,8 @@ import { metaAttribute, MetaConstants } from '~utils/metaAttribute'; import { useId } from '~utils/useId'; import { useVerifyAllowedChildren } from '~utils/useVerifyAllowedChildren'; import { makeAnalyticsAttribute } from '~utils/makeAnalyticsAttribute'; +import type { BladeElementRef } from '~utils/types'; +import { mergeRefs } from '~utils/useMergeRefs'; const SHOW_DRAWER = 'show-drawer'; @@ -65,18 +67,21 @@ const DrawerOverlay = styled(FloatingOverlay)(({ theme }) => { }; }); -const _Drawer = ({ - isOpen, - onDismiss, - zIndex = componentZIndices.drawer, - children, - accessibilityLabel, - showOverlay = true, - initialFocusRef, - isLazy = true, - testID, - ...rest -}: DrawerProps): React.ReactElement => { +const _Drawer: React.ForwardRefRenderFunction = ( + { + isOpen, + onDismiss, + zIndex = componentZIndices.drawer, + children, + accessibilityLabel, + showOverlay = true, + initialFocusRef, + isLazy = true, + testID, + ...rest + }, + ref, +) => { const closeButtonRef = React.useRef(null); const [zIndexState, setZIndexState] = React.useState(zIndex); @@ -155,7 +160,7 @@ const _Drawer = ({ {...makeAnalyticsAttribute(rest)} zIndex={zIndexState} > - {showOverlay || stackingLevel === 2 ? ( + {showOverlay ? ( { onDismiss(); @@ -184,7 +189,7 @@ const _Drawer = ({ height="100%" display="flex" flexDirection="column" - ref={refs.setFloating} + ref={mergeRefs(ref, refs.setFloating)} onKeyDown={(event) => { if (event?.key === 'Escape' || event?.code === 'Escape') { onDismiss(); @@ -238,7 +243,8 @@ const _Drawer = ({ * * */ -const Drawer = assignWithoutSideEffects(_Drawer, { +const Drawer = assignWithoutSideEffects(React.forwardRef(_Drawer), { + displayName: 'Drawer', componentId: drawerComponentIds.Drawer, }); From fbb78e80635dd315b9b7be0057532bd1e890dcb8 Mon Sep 17 00:00:00 2001 From: anuraghazra Date: Tue, 17 Dec 2024 02:10:57 +0530 Subject: [PATCH 4/8] chore: expose event from radio group onchange --- packages/blade/src/components/Radio/Radio.tsx | 4 ++-- .../src/components/Radio/RadioGroup/RadioGroup.tsx | 10 +++++++++- .../src/components/Radio/RadioGroup/useRadioGroup.ts | 10 ++++++---- packages/blade/src/components/Radio/useRadio.ts | 2 +- packages/blade/src/utils/useControllable.ts | 12 +++++++++--- 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/packages/blade/src/components/Radio/Radio.tsx b/packages/blade/src/components/Radio/Radio.tsx index 29272c69bd2..ba99e90aedc 100644 --- a/packages/blade/src/components/Radio/Radio.tsx +++ b/packages/blade/src/components/Radio/Radio.tsx @@ -88,9 +88,9 @@ const _Radio: React.ForwardRefRenderFunction = ( const isReactNative = getPlatformType() === 'react-native'; const _size = groupProps.size ?? size; - const handleChange: OnChange = ({ isChecked, value }) => { + const handleChange: OnChange = ({ isChecked, value, event }) => { if (isChecked) { - groupProps?.state?.setValue(value!); + groupProps?.state?.setValue(value!, event); } else { groupProps?.state?.removeValue(); } diff --git a/packages/blade/src/components/Radio/RadioGroup/RadioGroup.tsx b/packages/blade/src/components/Radio/RadioGroup/RadioGroup.tsx index e369248035a..84f843c69b5 100644 --- a/packages/blade/src/components/Radio/RadioGroup/RadioGroup.tsx +++ b/packages/blade/src/components/Radio/RadioGroup/RadioGroup.tsx @@ -76,7 +76,15 @@ type RadioGroupProps = { /** * The callback invoked when any of the radio's state changes */ - onChange?: ({ name, value }: { name: string | undefined; value: string }) => void; + onChange?: ({ + name, + value, + event, + }: { + name: string | undefined; + value: string; + event: React.ChangeEvent; + }) => void; /** * The name of the input field in a radio * (Useful for form submission). diff --git a/packages/blade/src/components/Radio/RadioGroup/useRadioGroup.ts b/packages/blade/src/components/Radio/RadioGroup/useRadioGroup.ts index 6f76e3560db..f8d6e61b1e2 100644 --- a/packages/blade/src/components/Radio/RadioGroup/useRadioGroup.ts +++ b/packages/blade/src/components/Radio/RadioGroup/useRadioGroup.ts @@ -22,7 +22,7 @@ type UseRadioGroupProps = Pick< export type State = { value: string; - setValue(value: string): void; + setValue(value: string, event: React.ChangeEvent): void; removeValue(): void; isChecked(value: string): boolean; }; @@ -52,18 +52,20 @@ const useRadioGroup = ({ const [checkedValue, setValue] = useControllableState({ value, defaultValue, - onChange: (v: string) => onChange?.({ value: v, name: fallbackName }), + onChange: (v, event) => { + onChange?.({ value: v, name: fallbackName, event }); + }, }); const state = React.useMemo(() => { return { value: checkedValue, - setValue(v: string): void { + setValue(v, event): void { if (isDisabled) { return; } - setValue(() => v); + setValue(() => v, false, event); }, removeValue(): void { if (isDisabled) { diff --git a/packages/blade/src/components/Radio/useRadio.ts b/packages/blade/src/components/Radio/useRadio.ts index 926bb148932..f7fbf806c83 100644 --- a/packages/blade/src/components/Radio/useRadio.ts +++ b/packages/blade/src/components/Radio/useRadio.ts @@ -14,7 +14,7 @@ export type OnChange = ({ value, }: { isChecked: boolean; - event?: React.ChangeEvent; + event: React.ChangeEvent; value?: string; }) => void; diff --git a/packages/blade/src/utils/useControllable.ts b/packages/blade/src/utils/useControllable.ts index aaa9a7da798..910b9126277 100644 --- a/packages/blade/src/utils/useControllable.ts +++ b/packages/blade/src/utils/useControllable.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ /* eslint-disable @typescript-eslint/explicit-module-boundary-types */ /* eslint-disable @typescript-eslint/explicit-function-return-type */ /* eslint-disable react-hooks/exhaustive-deps */ @@ -13,6 +14,11 @@ type ControllableStateSetter = ( * If `true`, `onChange` won't be called */ skipUpdate?: boolean, + /** + * Extra data to be passed to `onChange` callback + * Example use case: passing event object to `onChange` callback + */ + extraData?: any, ) => void; type UseControllableStateProps = { @@ -27,7 +33,7 @@ type UseControllableStateProps = { /** * The callback fired when the value changes */ - onChange?: (value: T) => void; + onChange?: (value: T, extraData: any) => void; shouldUpdate?: (prev: T, next: T) => boolean; }; @@ -57,13 +63,13 @@ function useControllableState(props: UseControllableStateProps) { const value = isControlled && typeof valueProp !== 'undefined' ? valueProp : valueState; const updateValue: ControllableStateSetter = useCallbackRef( - (next, skipUpdate = false) => { + (next, skipUpdate = false, extraData) => { const nextValue = next(value); if (!isControlled) setValue(nextValue); // We don't want to call onChange if skipUpdate is true or if the value is not changed if (!shouldUpdateProp(value, nextValue)) return; if (skipUpdate) return; - onChangeProp?.(nextValue); + onChangeProp?.(nextValue, extraData); }, [isControlled, onChangeProp, value, shouldUpdateProp], ); From cf5797f83faebe51266fb26dc907dbc3e4890f75 Mon Sep 17 00:00:00 2001 From: anuraghazra Date: Tue, 17 Dec 2024 02:18:16 +0530 Subject: [PATCH 5/8] chore: fix tests --- .../__tests__/ButtonGroup.web.test.tsx | 36 +------------------ .../src/utils/useControllable.web.test.tsx | 13 ++++--- 2 files changed, 9 insertions(+), 40 deletions(-) diff --git a/packages/blade/src/components/ButtonGroup/__tests__/ButtonGroup.web.test.tsx b/packages/blade/src/components/ButtonGroup/__tests__/ButtonGroup.web.test.tsx index e23f3a32d8a..2ca97964371 100644 --- a/packages/blade/src/components/ButtonGroup/__tests__/ButtonGroup.web.test.tsx +++ b/packages/blade/src/components/ButtonGroup/__tests__/ButtonGroup.web.test.tsx @@ -7,7 +7,6 @@ import { Button } from '~components/Button/Button'; import { ChevronDownIcon, PlusIcon } from '~components/Icons'; import { Dropdown, DropdownButton, DropdownOverlay } from '~components/Dropdown'; import { ActionList, ActionListItem } from '~components/ActionList'; -import { AutoComplete } from '~components/Input/DropdownInputTriggers'; beforeAll(() => jest.spyOn(console, 'error').mockImplementation()); afterAll(() => jest.restoreAllMocks()); @@ -168,40 +167,7 @@ describe('', () => { , ), ).toThrowError( - '[Blade: ButtonGroup]: Only "Button" or "Dropdown" component with Button trigger are allowed as children.', - ); - }); - - it('should throw error with invalid dropdown children', () => { - expect(() => - renderWithTheme( - - - - { - console.log({ name, values }); - }} - onInputValueChange={({ name, value }) => { - console.log({ name, value }); - }} - /> - - - - - - - - - - , - ), - ).toThrowError( - '[Blade: ButtonGroup]: Only "Button" or "Dropdown" component with Button trigger are allowed as children.', + '[Blade: ButtonGroup]: Only `Button, Dropdown, Tooltip, Popover` components are accepted in `ButtonGroup` children', ); }); }); diff --git a/packages/blade/src/utils/useControllable.web.test.tsx b/packages/blade/src/utils/useControllable.web.test.tsx index 12aeab24727..b1da0d96db3 100644 --- a/packages/blade/src/utils/useControllable.web.test.tsx +++ b/packages/blade/src/utils/useControllable.web.test.tsx @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ import userEvent from '@testing-library/user-event'; import React from 'react'; import renderWithTheme from './testing/renderWithTheme.web'; @@ -10,18 +11,20 @@ const Example = ({ }: { value?: number; defaultValue?: number; - onChange?: (value: number) => void; + onChange?: (value: number, extra: any) => void; }): React.ReactElement => { const [_value, setValue] = useControllableState({ value, defaultValue: defaultValue ?? 0, - onChange, + onChange: (value, extraData) => { + onChange?.(value, extraData); + }, }); return (