fix: Added rowGap and colGap as PropsToBeCssPrefixPrepended in Box component #38096
Conversation
WalkthroughThe changes in this pull request involve the addition of two new properties, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/packages/design-system/ads/src/Box/Box.constants.ts (1)
30-31: Note naming inconsistency with PR descriptionThe PR description uses "colGap" while the implementation uses "columnGap". While the implementation is correct (as it matches the CSS property name), this should be consistent in documentation.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/packages/design-system/ads/src/Box/Box.constants.ts(1 hunks)
🔇 Additional comments (2)
app/client/packages/design-system/ads/src/Box/Box.constants.ts (2)
30-31: Verify existing usage patterns
Let's check if these properties are already being used elsewhere in the codebase to ensure consistent implementation.
✅ Verification successful
Let me try a different approach to find any existing usage patterns. The previous command failed because of the file type specification, so let's try a broader search.
Properties are consistently used across the codebase
The rowGap and columnGap properties are already being used in multiple places:
- Design system components (Flex, Grid)
- Layout system utilities
- Mobile responsive tests
The addition of these properties to Box.constants.ts aligns with existing usage patterns and maintains consistency across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing usage of rowGap and columnGap in the codebase
rg -A 2 'rowGap|columnGap' --type typescript
Length of output: 81
Script:
#!/bin/bash
# Search for existing usage of rowGap and columnGap in the codebase
rg -i 'rowgap|columngap'
# Also search for specific styled-system props usage
rg '"rowGap"|"columnGap"'
Length of output: 4709
30-31: Verify browser compatibility for rowGap and columnGap
The addition of these properties is logical, but let's verify if they need vendor prefixes in modern browsers.
…mponent (appsmithorg#38096) ## Description rowGap and colGap was not accepting standard `spaces` values in Flex and Grid component. This PR fix that issue. Fixes appsmithorg#38097 ## Automation /ok-to-test tags="@tag.Sanity" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/12273259261> > Commit: 16b48de > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12273259261&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Wed, 11 Dec 2024 10:01:27 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced new CSS properties `rowGap` and `columnGap` for enhanced grid layout management within the design system. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
rowGap and colGap was not accepting standard
spacesvalues in Flex and Grid component. This PR fix that issue.Fixes #38097
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12273259261
Commit: 16b48de
Cypress dashboard.
Tags:
@tag.SanitySpec:
Wed, 11 Dec 2024 10:01:27 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
rowGapandcolumnGapfor enhanced grid layout management within the design system.