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

fix: button group loading #2544

Merged
merged 6 commits into from
Feb 17, 2025
+548 −443
Merged

fix: button group loading #2544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0922da1
fix: bug and code refactor
tewarig Feb 13, 2025
5e3612a
chore: border radius
tewarig Feb 13, 2025
59a6593
chore: ts fixes
tewarig Feb 13, 2025
3e278c1
chore: border radius changes
tewarig Feb 13, 2025
6763f3e
chore: update snap
tewarig Feb 13, 2025
bc674f5
fix: loading single button in buttonGroup
tewarig Feb 13, 2025
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
5 changes: 5 additions & 0 deletions .changeset/warm-fireants-nail.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@razorpay/blade': patch
---

fix(blade): loading single button in buttonGroup
4 changes: 3 additions & 1 deletion packages/blade/src/components/Button/BaseButton/BaseButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,9 @@ const _BaseButton: React.ForwardRefRenderFunction<BladeElementRef, BaseButtonPro
const isChildrenComponent = React.isValidElement(children);

// Button cannot be disabled when its rendered as Link
const disabled = buttonGroupProps.isDisabled ?? (isLoading || (isDisabled && !isLink));
// button should be allowed to be disabled in any case...
// either through button group or we should allow to disable an individual button
const disabled = buttonGroupProps.isDisabled || isLoading || (isDisabled && !isLink);

if (__DEV__) {
if (!Icon && !childrenString?.trim()) {
Expand Down
37 changes: 21 additions & 16 deletions packages/blade/src/components/ButtonGroup/StyledButtonGroup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,38 @@ import styled from 'styled-components';
import type { StyledButtonGroupProps } from './types';
import BaseBox from '~components/Box/BaseBox';
import { makeBorderSize } from '~utils';
import { getBackgroundColorToken } from '~components/Button/BaseButton/BaseButton';
import getIn from '~utils/lodashButBetter/get';

const StyledButtonGroup = styled(BaseBox)<StyledButtonGroupProps>(
({ theme, isDisabled, variant = 'primary', color, isFullWidth }) => {
({ theme, variant = 'primary', isFullWidth }) => {
return {
display: 'flex',
width: isFullWidth ? '100%' : 'fit-content',
borderWidth: makeBorderSize(theme.border.width.thin),
borderRadius: makeBorderSize(theme.border.radius.medium),
borderStyle: 'solid',
borderColor: getIn(
theme.colors,
getBackgroundColorToken({
// Only secondary variant has border a border, for other variants we use background color so that the border is not visible
property: variant === 'secondary' ? 'border' : 'background',
variant,
color,
state: isDisabled ? 'disabled' : 'default',
}),
),

borderColor: 'transparent',
overflow: 'hidden',
'button[role="button"]': {
borderRadius: 0,
border: 'none',
flex: isFullWidth ? 1 : 'auto',
borderRadius: 0,
},

...(variant === 'secondary' && {
'button[role="button"]:first-child': {
borderRight: 'none',
borderTopLeftRadius: makeBorderSize(theme.border.radius.medium),
borderBottomLeftRadius: makeBorderSize(theme.border.radius.medium),
},
'button[role="button"]:not(:first-child):not(:last-child)': {
borderLeft: 'none',
borderRight: 'none',
},
'button[role="button"]:last-child': {
borderLeft: 'none',
borderTopRightRadius: makeBorderSize(theme.border.radius.medium),
borderBottomRightRadius: makeBorderSize(theme.border.radius.medium),
},
Comment on lines +21 to +35
Copy link
Member

Choose a reason for hiding this comment

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

How is this related to loading? 🤔

Copy link
Contributor Author

@tewarig tewarig Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Earlier, we added a border to the Button Group container and removed the borders from all individual buttons.
However, when we enabled the loading state for a button, it looked like this:

Screenshot 2025-02-14 at 3 58 11 PM

I think the ideal way to handle this was to modify the border-radius of the buttons. So, I refactored the Button Group code a bit and also updated how the border is handled for the secondary variant.

Screenshot 2025-02-14 at 4 08 15 PM

}),
};
},
);
Expand Down
46 changes: 23 additions & 23 deletions ...es/blade/src/components/ButtonGroup/__tests__/__snapshots__/ButtonGroup.ssr.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<ButtonGroup /> should render ButtonGroup 1`] = `"<div id="root"><div data-blade-component="button-group" data-testid="button-group-test" role="group" class="BaseBox-bmPWx StyledButtonGroup-gr4xbj-0 jpCXAr"><button type="button" data-blade-component="button" role="button" class="StyledBaseButtonweb__StyledBaseButton-sc-26bt38-0 iCiokj"><div data-blade-component="base-box" class="BaseBox-bmPWx AnimatedButtonContentweb__AnimatedButtonContent-sc-1fkx0t6-0 eHcfsa"><div data-blade-component="base-box" class="BaseBox-bmPWx BaseButton__ButtonContent-zf1huq-0 LeezU esItiH"><div class="StyledBaseText-dVBfTO eGcQGV" data-blade-component="base-text">One</div></div></div></button><div data-blade-component="base-box" class="BaseBox-bmPWx ButtonGroupweb__StyledDivider-sc-1lc1evg-0 cbYKkw"></div><button type="button" data-blade-component="button" role="button" class="StyledBaseButtonweb__StyledBaseButton-sc-26bt38-0 iCiokj"><div data-blade-component="base-box" class="BaseBox-bmPWx AnimatedButtonContentweb__AnimatedButtonContent-sc-1fkx0t6-0 eHcfsa"><div data-blade-component="base-box" class="BaseBox-bmPWx BaseButton__ButtonContent-zf1huq-0 LeezU esItiH"><div class="StyledBaseText-dVBfTO eGcQGV" data-blade-component="base-text">Two</div></div></div></button><div data-blade-component="base-box" class="BaseBox-bmPWx ButtonGroupweb__StyledDivider-sc-1lc1evg-0 cbYKkw"></div><button type="button" data-blade-component="button" role="button" class="StyledBaseButtonweb__StyledBaseButton-sc-26bt38-0 iCiokj"><div data-blade-component="base-box" class="BaseBox-bmPWx AnimatedButtonContentweb__AnimatedButtonContent-sc-1fkx0t6-0 eHcfsa"><div data-blade-component="base-box" class="BaseBox-bmPWx BaseButton__ButtonContent-zf1huq-0 LeezU esItiH"><div class="StyledBaseText-dVBfTO eGcQGV" data-blade-component="base-text">Three</div></div></div></button></div></div>"`;
exports[`<ButtonGroup /> should render ButtonGroup 1`] = `"<div id="root"><div data-blade-component="button-group" data-testid="button-group-test" role="group" class="BaseBox-bmPWx StyledButtonGroup-gr4xbj-0 hxzFGp"><button type="button" data-blade-component="button" role="button" class="StyledBaseButtonweb__StyledBaseButton-sc-26bt38-0 iCiokj"><div data-blade-component="base-box" class="BaseBox-bmPWx AnimatedButtonContentweb__AnimatedButtonContent-sc-1fkx0t6-0 eHcfsa"><div data-blade-component="base-box" class="BaseBox-bmPWx BaseButton__ButtonContent-zf1huq-0 LeezU esItiH"><div class="StyledBaseText-dVBfTO eGcQGV" data-blade-component="base-text">One</div></div></div></button><div data-blade-component="base-box" class="BaseBox-bmPWx ButtonGroupweb__StyledDivider-sc-1lc1evg-0 cbYKkw"></div><button type="button" data-blade-component="button" role="button" class="StyledBaseButtonweb__StyledBaseButton-sc-26bt38-0 iCiokj"><div data-blade-component="base-box" class="BaseBox-bmPWx AnimatedButtonContentweb__AnimatedButtonContent-sc-1fkx0t6-0 eHcfsa"><div data-blade-component="base-box" class="BaseBox-bmPWx BaseButton__ButtonContent-zf1huq-0 LeezU esItiH"><div class="StyledBaseText-dVBfTO eGcQGV" data-blade-component="base-text">Two</div></div></div></button><div data-blade-component="base-box" class="BaseBox-bmPWx ButtonGroupweb__StyledDivider-sc-1lc1evg-0 cbYKkw"></div><button type="button" data-blade-component="button" role="button" class="StyledBaseButtonweb__StyledBaseButton-sc-26bt38-0 iCiokj"><div data-blade-component="base-box" class="BaseBox-bmPWx AnimatedButtonContentweb__AnimatedButtonContent-sc-1fkx0t6-0 eHcfsa"><div data-blade-component="base-box" class="BaseBox-bmPWx BaseButton__ButtonContent-zf1huq-0 LeezU esItiH"><div class="StyledBaseText-dVBfTO eGcQGV" data-blade-component="base-text">Three</div></div></div></button></div></div>"`;

exports[`<ButtonGroup /> should render ButtonGroup 2`] = `
.c3.c3.c3.c3.c3 {
Expand All @@ -25,6 +25,28 @@ exports[`<ButtonGroup /> should render ButtonGroup 2`] = `
z-index: 1;
}

.c0.c0.c0.c0.c0 {
display: -webkit-box;
display: -webkit-flex;
display: -ms-flexbox;
display: flex;
width: -webkit-fit-content;
width: -moz-fit-content;
width: fit-content;
border-width: 1px;
border-radius: 4px;
border-style: solid;
border-color: transparent;
overflow: hidden;
}

.c0.c0.c0.c0.c0 button[role="button"] {
-webkit-flex: auto;
-ms-flex: auto;
flex: auto;
border-radius: 0;
}

.c1.c1.c1.c1.c1 {
min-height: 36px;
width: auto;
Expand Down Expand Up @@ -120,28 +142,6 @@ exports[`<ButtonGroup /> should render ButtonGroup 2`] = `
opacity: 1;
}

.c0.c0.c0.c0.c0 {
display: -webkit-box;
display: -webkit-flex;
display: -ms-flexbox;
display: flex;
width: -webkit-fit-content;
width: -moz-fit-content;
width: fit-content;
border-width: 1px;
border-radius: 4px;
border-style: solid;
border-color: hsla(227,100%,59%,1);
}

.c0.c0.c0.c0.c0 button[role="button"] {
border-radius: 0;
border: none;
-webkit-flex: auto;
-ms-flex: auto;
flex: auto;
}

.c6.c6.c6.c6.c6 {
border-width: 0;
border-left-style: solid;
Expand Down
Loading
Loading