Skip to content
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.

Sign up for GitHub

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

feature: add props isSplit to button with dropdown #40278

Merged
merged 11 commits into from
Apr 29, 2024
Merged

feature: add props isSplit to button with dropdown #40278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
65e1251
feature: add props isSplit to button with dropdown
nkdengineer Apr 16, 2024
98fef23
use useCallBack to get icon right
nkdengineer Apr 16, 2024
74ae64d
Merge branch 'main' of https://github.com/nkdengineer/App into fix/39832
nkdengineer Apr 17, 2024
559d293
Merge branch 'fix/39832' of https://github.com/nkdengineer/App into f…
nkdengineer Apr 17, 2024
a9d6b4b
fix add custom icon right button
nkdengineer Apr 17, 2024
d80eeb7
Merge branch 'main' into fix/39832
nkdengineer Apr 23, 2024
0997b0a
Use more reasonable variable names
nkdengineer Apr 23, 2024
7d9777f
Merge branch 'main' into fix/39832
nkdengineer Apr 25, 2024
b47c929
fix: remmove custom right icon button
nkdengineer Apr 25, 2024
3fc5ecf
fix lint
nkdengineer Apr 26, 2024
7ce8eed
Merge branch 'main' into fix/39832
nkdengineer Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions src/components/Button/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ type ButtonProps = Partial<ChildrenProps> & {

/** Boolean whether to display the right icon */
shouldShowRightIcon?: boolean;

/** Whether the button should use split style or not */
isSplitButton?: boolean;
};

type KeyboardShortcutComponentProps = Pick<ButtonProps, 'isDisabled' | 'isLoading' | 'onPress' | 'pressOnEnter' | 'allowBubble' | 'enterKeyEventListenerPriority'>;
Expand Down Expand Up @@ -198,6 +201,7 @@ function Button(

id = '',
accessibilityLabel = '',
isSplitButton = false,
...rest
}: ButtonProps,
ref: ForwardedRef<View | HTMLDivElement>,
Expand Down Expand Up @@ -253,13 +257,22 @@ function Button(
</View>
{shouldShowRightIcon && (
<View style={[styles.justifyContentCenter, large ? styles.ml2 : styles.ml1, iconRightStyles]}>
<Icon
src={iconRight}
fill={iconFill ?? (success || danger ? theme.textLight : theme.icon)}
small={small}
medium={medium}
large={large}
/>
{!isSplitButton ? (
<Icon
src={iconRight}
fill={iconFill ?? (success || danger ? theme.textLight : theme.icon)}
small={medium}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was it set like that? is there any specific reason? Right now I need to add small icon handling and I'll adjust it in this way:

small={small}
medium={medium}
large={large}

may it broke something?
@nkdengineer

medium={large}
/>
) : (
<Icon
src={iconRight}
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
fill={iconFill ?? (success || danger ? theme.textLight : theme.icon)}
small={small}
medium={medium}
large={large}
/>
)}
</View>
)}
</View>
Expand Down
63 changes: 35 additions & 28 deletions src/components/ButtonWithDropdownMenu/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {ButtonWithDropdownMenuProps} from './types';

function ButtonWithDropdownMenu<IValueType>({
success = false,
isSplitButton = true,
isLoading = false,
isDisabled = false,
pressOnEnter = false,
Expand All @@ -40,7 +41,7 @@ function ButtonWithDropdownMenu<IValueType>({
const [isMenuVisible, setIsMenuVisible] = useState(false);
const [popoverAnchorPosition, setPopoverAnchorPosition] = useState<AnchorPosition | null>(null);
const {windowWidth, windowHeight} = useWindowDimensions();
const caretButton = useRef<View & HTMLDivElement>(null);
const caretButton = useRef<View | HTMLDivElement | null>(null);
const selectedItem = options[selectedItemIndex] || options[0];
const innerStyleDropButton = StyleUtils.getDropDownButtonHeight(buttonSize);
const isButtonSizeLarge = buttonSize === CONST.DROPDOWN_BUTTON_SIZE.LARGE;
Expand All @@ -64,51 +65,57 @@ function ButtonWithDropdownMenu<IValueType>({
});
}
}, [windowWidth, windowHeight, isMenuVisible, anchorAlignment.vertical]);

return (
<View style={wrapperStyle}>
{shouldAlwaysShowDropdownMenu || options.length > 1 ? (
<View style={[styles.flexRow, styles.justifyContentBetween, styles.alignItemsCenter, style]}>
<Button
success={success}
pressOnEnter={pressOnEnter}
ref={buttonRef}
onPress={(event) => onPress(event, selectedItem.value)}
ref={(ref) => {
caretButton.current = ref;
Copy link
Member

@rushatgabhane rushatgabhane Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR caused a bug - Tapping Pay button in an invoice crashes the app

Fixed here - #44919 (comment)

}}
Comment on lines +75 to +77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This caused style regression
More details (root cause & solution): #46127 (comment)

onPress={(event) => (!isSplitButton ? setIsMenuVisible(!isMenuVisible) : onPress(event, selectedItem.value))}
text={customText ?? selectedItem.text}
isDisabled={isDisabled || !!selectedItem.disabled}
isLoading={isLoading}
shouldRemoveRightBorderRadius
style={[styles.flex1, styles.pr0]}
large={isButtonSizeLarge}
medium={!isButtonSizeLarge}
innerStyles={[innerStyleDropButton, customText !== undefined && styles.cursorDefault, customText !== undefined && styles.pointerEventsNone]}
innerStyles={[innerStyleDropButton, !isSplitButton && styles.dropDownButtonCartIconView]}
enterKeyEventListenerPriority={enterKeyEventListenerPriority}
iconRight={Expensicons.DownArrow}
shouldShowRightIcon={!isSplitButton}
isSplitButton={isSplitButton}
/>

<Button
ref={caretButton}
success={success}
isDisabled={isDisabled}
style={[styles.pl0]}
onPress={() => setIsMenuVisible(!isMenuVisible)}
shouldRemoveLeftBorderRadius
large={isButtonSizeLarge}
medium={!isButtonSizeLarge}
innerStyles={[styles.dropDownButtonCartIconContainerPadding, innerStyleDropButton]}
enterKeyEventListenerPriority={enterKeyEventListenerPriority}
>
<View style={[styles.dropDownButtonCartIconView, innerStyleDropButton]}>
<View style={[success ? styles.buttonSuccessDivider : styles.buttonDivider]} />
<View style={[isButtonSizeLarge ? styles.dropDownLargeButtonArrowContain : styles.dropDownMediumButtonArrowContain]}>
<Icon
medium={isButtonSizeLarge}
small={!isButtonSizeLarge}
src={Expensicons.DownArrow}
fill={success ? theme.buttonSuccessText : theme.icon}
/>
{isSplitButton && (
<Button
ref={caretButton}
success={success}
isDisabled={isDisabled}
style={[styles.pl0]}
onPress={() => setIsMenuVisible(!isMenuVisible)}
shouldRemoveLeftBorderRadius
large={isButtonSizeLarge}
medium={!isButtonSizeLarge}
innerStyles={[styles.dropDownButtonCartIconContainerPadding, innerStyleDropButton]}
enterKeyEventListenerPriority={enterKeyEventListenerPriority}
>
<View style={[styles.dropDownButtonCartIconView, innerStyleDropButton]}>
<View style={[success ? styles.buttonSuccessDivider : styles.buttonDivider]} />
<View style={[isButtonSizeLarge ? styles.dropDownLargeButtonArrowContain : styles.dropDownMediumButtonArrowContain]}>
<Icon
medium={isButtonSizeLarge}
small={!isButtonSizeLarge}
src={Expensicons.DownArrow}
fill={success ? theme.buttonSuccessText : theme.icon}
/>
</View>
</View>
</View>
</Button>
</Button>
)}
</View>
) : (
<Button
Expand Down
3 changes: 3 additions & 0 deletions src/components/ButtonWithDropdownMenu/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ type ButtonWithDropdownMenuProps<TValueType> = {

/** Additional style to add to the wrapper */
wrapperStyle?: StyleProp<ViewStyle>;

/** Whether the button should use split style or not */
isSplitButton?: boolean;
};

export type {PaymentType, WorkspaceMemberBulkActionType, WorkspaceDistanceRatesBulkActionType, DropdownOption, ButtonWithDropdownMenuProps, WorkspaceTaxRatesBulkActionType};
1 change: 1 addition & 0 deletions src/pages/workspace/WorkspaceMembersPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ function WorkspaceMembersPage({personalDetails, invitedEmailsToAccountIDsDraft,
onPress={() => null}
options={getBulkActionsButtonOptions()}
buttonRef={dropdownButtonRef}
isSplitButton={false}
style={[isSmallScreenWidth && styles.flexGrow1]}
/>
) : (
Expand Down
1 change: 1 addition & 0 deletions src/pages/workspace/categories/WorkspaceCategoriesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ function WorkspaceCategoriesPage({policy, route}: WorkspaceCategoriesPageProps)
buttonSize={CONST.DROPDOWN_BUTTON_SIZE.MEDIUM}
customText={translate('workspace.common.selected', {selectedNumber: selectedCategoriesArray.length})}
options={options}
isSplitButton={false}
style={[isSmallScreenWidth && styles.flexGrow1, isSmallScreenWidth && styles.mb3]}
/>
);
Expand Down
1 change: 1 addition & 0 deletions src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ function PolicyDistanceRatesPage({policy, route}: PolicyDistanceRatesPageProps)
buttonRef={dropdownButtonRef}
style={[isSmallScreenWidth && styles.flexGrow1]}
wrapperStyle={styles.w100}
isSplitButton={false}
/>
)}
</View>
Expand Down
1 change: 1 addition & 0 deletions src/pages/workspace/tags/WorkspaceTagsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ function WorkspaceTagsPage({policyTags, route}: WorkspaceTagsPageProps) {
onPress={() => null}
shouldAlwaysShowDropdownMenu
pressOnEnter
isSplitButton={false}
buttonSize={CONST.DROPDOWN_BUTTON_SIZE.MEDIUM}
customText={translate('workspace.common.selected', {selectedNumber: selectedTagsArray.length})}
options={options}
Expand Down
1 change: 1 addition & 0 deletions src/pages/workspace/taxes/WorkspaceTaxesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ function WorkspaceTaxesPage({
customText={translate('workspace.common.selected', {selectedNumber: selectedTaxesIDs.length})}
shouldAlwaysShowDropdownMenu
pressOnEnter
isSplitButton={false}
style={[isSmallScreenWidth && styles.w50, isSmallScreenWidth && styles.mb3]}
/>
);
Expand Down
Loading