fix(blade): chipgroup grid alignment #2536

tewarig · 2025-02-09T18:35:19Z

Description

ref - https://razorpay.slack.com/archives/CMQ3RBHEU/p1738752395317139

Changes

Additional Information

Component Checklist

Update Component Status Page
Perform Manual Testing in Other Browsers
Add KitchenSink Story
Add Interaction Tests (if applicable)
Add changeset

changeset-bot · 2025-02-09T18:35:25Z

🦋 Changeset detected

Latest commit: a1f66f9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@razorpay/blade	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2025-02-09T18:35:31Z

✅ PR title follows Conventional Commits specification.

codesandbox-ci · 2025-02-09T18:38:53Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a1f66f9:

Sandbox	Source
razorpay/blade: basic	Configuration

rzpcibot · 2025-02-09T18:51:52Z

Bundle Size Report

Updated Components

Status	Component	Base Size (kb)	Current Size (kb)	Diff
⬆	Chip, ChipGroup	8.486	8.530	+0.044 KB
⬆	DatePicker	87.068	87.119	+0.051 KB
⬆	Indicator	1.087	1.089	+0.002 KB

Generated by 🚫 dangerJS against a1f66f9

anuraghazra · 2025-02-10T07:02:54Z

We should ideally do this, and let consumer handle the layout.

      <ChipGroupComponent
        selectionType="single"
        label="Select a gift card with value (custom chipGroupContainerLayout)"
      >
        <Box display="grid" gridTemplateColumns="1fr 1fr 1fr">
          {chipArray.map((chip, index) => (
            <ChipComponent key={index} value={chip.value}>
              {chip.label}
            </ChipComponent>
          ))}
        </Box>
      </ChipGroupComponent>

Only challenge is internally we are applying some default margins to each Chip component, we need to somehow make it work with the above example + keep the existing behaviour intact.

Maybe we could do something like:

            <BaseBox
              display="flex"
              flexDirection="row"
              flexWrap="wrap"
              rowGap={chipGroupGapTokens[size].bottom}
              columnGap={chipGroupGapTokens[size].right}
              marginBottom={chipGroupGapTokens[size].bottom}
            >
              {React.Children.map(children, (child, index) => {
                return <BaseBox key={index}>{child}</BaseBox>; // maybe we can remove the box if needed
              })}
            </BaseBox>

tewarig · 2025-02-10T09:52:30Z

We should ideally do this, and let consumer handle the layout.

      <ChipGroupComponent
        selectionType="single"
        label="Select a gift card with value (custom chipGroupContainerLayout)"
      >
        <Box display="grid" gridTemplateColumns="1fr 1fr 1fr">
          {chipArray.map((chip, index) => (
            <ChipComponent key={index} value={chip.value}>
              {chip.label}
            </ChipComponent>
          ))}
        </Box>
      </ChipGroupComponent>

Only challenge is internally we are applying some default margins to each Chip component, we need to somehow make it work with the above example + keep the existing behaviour intact.

Maybe we could do something like:

            <BaseBox
              display="flex"
              flexDirection="row"
              flexWrap="wrap"
              rowGap={chipGroupGapTokens[size].bottom}
              columnGap={chipGroupGapTokens[size].right}
              marginBottom={chipGroupGapTokens[size].bottom}
            >
              {React.Children.map(children, (child, index) => {
                return <BaseBox key={index}>{child}</BaseBox>; // maybe we can remove the box if needed
              })}
            </BaseBox>

thanks for suggesting this... i have implemented this... the api looks like

  <ChipGroupComponent
        selectionType="single"
        label="Select a gift card with value (custom chipGroupContainerLayout)"
      >
        <Box
          display="grid"
          gridTemplateColumns="repeat(3, minmax(0,120px))"
          gridTemplateRows="repeat(3, minmax(0,30px))"
          gap="spacing.3"
        >
          {chipArray.map((chip, index) => (
            <ChipComponent key={index} value={chip.value} width="120px">
              {chip.label}
            </ChipComponent>
          ))}
        </Box>
      </ChipGroupComponent>

also regarding that margin ... i think we don't need to do anything .. we can get children's from wrapper and render them.

   {React.Children.map(getChipWrapperChildren(), (child, index) => {
                return (
                  <BaseBox
                    key={index}
                    marginBottom={chipGroupGapTokens[size].bottom}
                    marginRight={chipGroupGapTokens[size].right}
                  >
                    {child}
                  </BaseBox>
                );
              })}

anuraghazra · 2025-02-10T11:39:46Z

packages/blade/src/components/Chip/ChipGroup.tsx

@@ -68,6 +70,24 @@ const _ChipGroup = (
      });
    }
  }
+  const isWrapperLayoutControlled =
+    typeof children === 'object' && getComponentId(children) === 'Box';


<BaseBox display="flex" flexDirection="row" flexWrap="wrap" rowGap={chipGroupGapTokens[size].bottom} columnGap={chipGroupGapTokens[size].right} marginBottom={chipGroupGapTokens[size].bottom} > {React.Children.map(children, (child, index) => { return <BaseBox key={index}>{child}</BaseBox>; // maybe we can remove the box if needed })} </BaseBox>

This code i shared, should just work without doing any of these checks/conditionals.

Yes, but I think we should add a wrapper and check for styles since consumers can pass props like background color or margin and padding, which should not be supported.

We should not do arbitrary checks if it's absolutely not necessary. If they add background etc anyways it will look odd.

Your above code will fail in many cases like so:

const SomeCustomWrapperBox = styled(Box)`` <ChipGroup> <SomeCustomWrapperBox> ... </SomeCustomWrapperBox> </ChipGroup> ``

packages/blade/src/components/Chip/types.ts

packages/blade/src/components/Chip/utils.ts

anuraghazra · 2025-02-11T06:18:18Z

packages/blade/src/components/Chip/ChipGroup.tsx

            <BaseBox display="flex" flexDirection="row" flexWrap="wrap">
-              {React.Children.map(children, (child, index) => {
-                return (
-                  <BaseBox
-                    key={index}
-                    marginBottom={chipGroupGapTokens[size].bottom}
-                    marginRight={chipGroupGapTokens[size].right}
-                  >
-                    {child}
-                  </BaseBox>
-                );
-              })}
+              {React.Children.map(children, (child) => child)}


Again, we need to keep the row gap etc props to make sure we are not changing the existing spacing that was there PLUS we let consumers handle the layout if they want.

<BaseBox display="flex" flexDirection="row" flexWrap="wrap" // Keep existing spacing that was there rowGap={chipGroupGapTokens[size].bottom} columnGap={chipGroupGapTokens[size].right} marginBottom={chipGroupGapTokens[size].bottom} > {React.Children.map(children, (child, index) => { return <BaseBox key={index}>{child}</BaseBox>; })} </BaseBox>

https://github.com/razorpay/blade/blob/98296fb7f8da7dfbb9a9db44e5b7e0fa957ddf51/packages/blade/src/components/Chip/Chip.tsx
Moved that to Chip, consumers can modify it from there.
Adding spacing on the wrapper won't work since this spacing needs to be applied to Chip.

@tewarig reusable components should not have margins, they are bit harmful. If in future we want to expose Chip as a standalone component (eg: Clickable only chip) then it will break component encapsulation.

Adding spacing on the wrapper won't work since this spacing needs to be applied to Chip.

Thus the BaseBox i've added on my example.

makes sense

anuraghazra · 2025-02-11T07:45:23Z

packages/blade/src/components/Chip/ChipGroup.tsx

+              columnGap={chipGroupGapTokens[size].right}
+              marginBottom={chipGroupGapTokens[size].bottom}
+            >
+              {React.Children.map(children, (child) => child)}


Probably can just be {children}

anuraghazra · 2025-02-11T07:48:10Z

packages/blade/src/components/Chip/ChipGroup.stories.tsx

+          gridTemplateColumns="repeat(3, minmax(0,120px))"
+          gridTemplateRows="repeat(3, minmax(0,30px))"
+          gap="spacing.3"
+        >
+          {chipArray.map((chip, index) => (
+            <ChipComponent key={index} value={chip.value} width="120px">


🤔 We shouldn't need to add explicit pixel values in ChipComponent.

ideally width=100% or flex={1} should work, instead of hard coding 120px like so:

<ChipComponent key={index} value={chip.value} width="100%" />

I would assume this would work and the chip should automatically stretch/flex to available space.

Not to mention explicit pixel values will break the layout if there is no space:

saurabhdaware · 2025-02-11T07:55:30Z

packages/blade/src/components/Box/styledProps/getStyledProps.ts

@@ -144,5 +144,5 @@ const getStyledProps = (props: Record<string, any>): StyledPropsBlade => {
  return removeUndefinedStyledProps(makeStyledProps(props));
 };

-export type { StyledPropsBlade };
+export type { StyledPropsBlade, StyledPropsInputType };


where is this used?

saurabhdaware · 2025-02-11T07:55:36Z

packages/blade/src/components/Box/BaseBox/types/propsTypes.ts

@@ -332,5 +332,5 @@ type BoxRefType = Platform.Select<{
  native: View;
 }>;

-export type { BaseBoxProps, BoxProps, BoxRefType, StyledPropsBlade, FlexboxProps };
+export type { BaseBoxProps, BoxProps, BoxRefType, StyledPropsBlade, FlexboxProps, GridProps };


where is this used?

saurabhdaware · 2025-02-11T07:56:25Z

packages/blade/src/components/Chip/Chip.tsx

@@ -39,7 +39,7 @@ type OnChange = ({
 }) => void;

 const _Chip: React.ForwardRefRenderFunction<BladeElementRef, ChipProps> = (
-  { isDisabled, value, children, icon: Icon, color, testID, _motionMeta, ...rest },
+  { isDisabled, value, children, icon: Icon, color, testID, _motionMeta, width, ...rest },


we should expose minWidth, maxWidth as well right if we're exposing width?

tewarig added 2 commits February 10, 2025 00:02

chore: expose grid and flexbox props

39da608

fix: add changeset

67b9d5f

tewarig requested review from chaitanyadeorukhkar, kamleshchandnani, anuraghazra, snitin315 and saurabhdaware as code owners February 9, 2025 18:35

chore: update lint error

fb84e53

tewarig added 2 commits February 10, 2025 12:45

feat: add width prop in chip

2481620

feat: let user change the layout

1e89e97

anuraghazra reviewed Feb 10, 2025

View reviewed changes

packages/blade/src/components/Chip/types.ts Outdated Show resolved Hide resolved

chore: remove unsed commit types

72efaeb

tewarig force-pushed the fix/chipgroup-grid-alignment branch from 177d4f9 to 72efaeb Compare February 10, 2025 11:48

tewarig added 2 commits February 11, 2025 01:32

chore: review changes

18838c7

chore: update snap

464acc8

anuraghazra reviewed Feb 11, 2025

View reviewed changes

packages/blade/src/components/Chip/utils.ts Outdated Show resolved Hide resolved

chore: removed unused utils

f280c95

anuraghazra reviewed Feb 11, 2025

View reviewed changes

tewarig added 4 commits February 11, 2025 11:48

chore: remove types

98296fb

chore: update ts

7c9803a

chore: review change

f3633f3

chore: update snap

a818274

anuraghazra reviewed Feb 11, 2025

View reviewed changes

saurabhdaware reviewed Feb 11, 2025

View reviewed changes

tewarig added 7 commits February 11, 2025 13:54

chore : review change

dc6fa5a

chore: review changes

23ee578

chore: addMaxWidth , minWidth

f4652cb

chore: expose maxWidth , minWidth

bb40a3f

chore: update chipgroup width

62f74c1

chore: more refactor

2a32ed1

chore: update snap

a1f66f9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(blade): chipgroup grid alignment #2536

fix(blade): chipgroup grid alignment #2536

tewarig commented Feb 9, 2025 •

edited

Loading

changeset-bot bot commented Feb 9, 2025 •

edited

Loading

github-actions bot commented Feb 9, 2025 •

edited

Loading

codesandbox-ci bot commented Feb 9, 2025 •

edited

Loading

rzpcibot commented Feb 9, 2025 •

edited

Loading

anuraghazra commented Feb 10, 2025 •

edited

Loading

tewarig commented Feb 10, 2025 •

edited

Loading

anuraghazra Feb 10, 2025

tewarig Feb 10, 2025

anuraghazra Feb 10, 2025

anuraghazra Feb 11, 2025

tewarig Feb 11, 2025 •

edited

Loading

anuraghazra Feb 11, 2025 •

edited

Loading

tewarig Feb 11, 2025

anuraghazra Feb 11, 2025

anuraghazra Feb 11, 2025 •

edited

Loading

saurabhdaware Feb 11, 2025

tewarig Feb 11, 2025

saurabhdaware Feb 11, 2025

tewarig Feb 11, 2025

saurabhdaware Feb 11, 2025

fix(blade): chipgroup grid alignment #2536

Are you sure you want to change the base?

fix(blade): chipgroup grid alignment #2536

Conversation

tewarig commented Feb 9, 2025 • edited Loading

Description

Changes

Additional Information

Component Checklist

changeset-bot bot commented Feb 9, 2025 • edited Loading

🦋 Changeset detected

github-actions bot commented Feb 9, 2025 • edited Loading

codesandbox-ci bot commented Feb 9, 2025 • edited Loading

rzpcibot commented Feb 9, 2025 • edited Loading

Bundle Size Report

anuraghazra commented Feb 10, 2025 • edited Loading

tewarig commented Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tewarig Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

anuraghazra Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anuraghazra Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tewarig commented Feb 9, 2025 •

edited

Loading

changeset-bot bot commented Feb 9, 2025 •

edited

Loading

github-actions bot commented Feb 9, 2025 •

edited

Loading

codesandbox-ci bot commented Feb 9, 2025 •

edited

Loading

rzpcibot commented Feb 9, 2025 •

edited

Loading

anuraghazra commented Feb 10, 2025 •

edited

Loading

tewarig commented Feb 10, 2025 •

edited

Loading

tewarig Feb 11, 2025 •

edited

Loading

anuraghazra Feb 11, 2025 •

edited

Loading

anuraghazra Feb 11, 2025 •

edited

Loading