Skip to content

Commit 42f8403

Browse files
laurkimAveline Thelinalex-page
committed
[Layout foundations] Refactor margin and padding logic
Including the `margin/padding` property was causing overrides when combining with `marginTop` or `paddingLeft`, for example. Co-authored-by: Aveline Thelin <[email protected]> Co-authored-by: Alex Page <[email protected]>
1 parent 334a236 commit 42f8403

File tree

2 files changed

+60
-42
lines changed

2 files changed

+60
-42
lines changed

polaris-react/src/components/Box/Box.scss

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,15 @@
11
.root {
2-
--pc-box-margin-bottom: initial;
3-
--pc-box-margin-left: initial;
4-
--pc-box-margin-right: initial;
5-
--pc-box-margin-top: initial;
6-
--pc-box-padding-bottom: initial;
7-
--pc-box-padding-left: initial;
8-
--pc-box-padding-right: initial;
9-
--pc-box-padding-top: initial;
102
display: block;
11-
margin-bottom: var(--pc-box-margin-bottom);
12-
margin-left: var(--pc-box-margin-left);
13-
margin-right: var(--pc-box-margin-right);
14-
margin-top: var(--pc-box-margin-top);
15-
padding-bottom: var(--pc-box-padding-bottom);
16-
padding-left: var(--pc-box-padding-left);
17-
padding-right: var(--pc-box-padding-right);
18-
padding-top: var(--pc-box-padding-top);
3+
/* stylelint-disable declaration-block-no-redundant-longhand-properties */
4+
margin-bottom: var(--pc-box-margin-bottom, initial);
5+
margin-left: var(--pc-box-margin-left, initial);
6+
margin-right: var(--pc-box-margin-right, initial);
7+
margin-top: var(--pc-box-margin-top, initial);
8+
padding-bottom: var(--pc-box-padding-bottom, initial);
9+
padding-left: var(--pc-box-padding-left, initial);
10+
padding-right: var(--pc-box-padding-right, initial);
11+
padding-top: var(--pc-box-padding-top, initial);
12+
/* stylelint-enable declaration-block-no-redundant-longhand-properties */
1913
}
2014

2115
.background {

polaris-react/src/components/Box/Box.tsx

Lines changed: 50 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import React, {ReactNode, forwardRef} from 'react';
22
import type * as Polymorphic from '@radix-ui/react-polymorphic';
3+
import type {spacing} from '@shopify/polaris-tokens';
34

45
import {classNames} from '../../utilities/css';
56

67
import styles from './Box.scss';
7-
import type {spacing} from '@shopify/polaris-tokens';
88

99
type Background =
1010
| 'background'
@@ -48,10 +48,18 @@ type Shadow = 'default' | 'transparent' | 'faint' | 'deep';
4848
type SpacingTokenGroup = typeof spacing;
4949
type SpacingTokenName = keyof SpacingTokenGroup;
5050

51+
// TODO: Bring this logic into tokens
5152
type SpacingTokenScale = SpacingTokenName extends `space-${infer Scale}`
5253
? Scale
5354
: never;
5455

56+
type Spacing = {
57+
bottom: SpacingTokenScale | '';
58+
left: SpacingTokenScale | '';
59+
right: SpacingTokenScale | '';
60+
top: SpacingTokenScale | '';
61+
};
62+
5563
interface BoxBaseProps {
5664
/** Background color of the Box */
5765
background?: Background;
@@ -94,41 +102,57 @@ export const Box = forwardRef(
94102
background,
95103
borderRadius,
96104
children,
97-
margin,
98-
marginBottom,
99-
marginLeft,
100-
marginRight,
101-
marginTop,
102-
padding,
103-
paddingBottom,
104-
paddingLeft,
105-
paddingRight,
106-
paddingTop,
105+
margin = '',
106+
marginBottom = '',
107+
marginLeft = '',
108+
marginRight = '',
109+
marginTop = '',
110+
padding = '',
111+
paddingBottom = '',
112+
paddingLeft = '',
113+
paddingRight = '',
114+
paddingTop = '',
107115
shadow,
108116
},
109117
ref,
110118
) => {
119+
const margins = {
120+
bottom: marginBottom ? marginBottom : margin,
121+
left: marginLeft ? marginLeft : margin,
122+
right: marginRight ? marginRight : margin,
123+
top: marginTop ? marginTop : margin,
124+
} as Spacing;
125+
126+
const paddings = {
127+
bottom: paddingBottom ? paddingBottom : padding,
128+
left: paddingLeft ? paddingLeft : padding,
129+
right: paddingRight ? paddingRight : padding,
130+
top: paddingTop ? paddingTop : padding,
131+
} as Spacing;
132+
111133
const style = {
112-
'--pc-box-margin': margin ? `var(--p-space-${margin})` : '',
113-
'--pc-box-margin-bottom': marginBottom
114-
? `var(--p-space-${marginBottom})`
134+
'--pc-box-margin-bottom': margins.bottom
135+
? `var(--p-space-${margins.bottom})`
136+
: '',
137+
'--pc-box-margin-left': margins.left
138+
? `var(--p-space-${margins.left})`
139+
: '',
140+
'--pc-box-margin-right': margins.right
141+
? `var(--p-space-${margins.right})`
115142
: '',
116-
'--pc-box-margin-left': marginLeft ? `var(--p-space-${marginLeft})` : '',
117-
'--pc-box-margin-right': marginRight
118-
? `var(--p-space-${marginRight})`
143+
'--pc-box-margin-top': margins.top ? `var(--p-space-${margins.top})` : '',
144+
'--pc-box-padding-bottom': paddings.bottom
145+
? `var(--p-space-${paddings.bottom})`
119146
: '',
120-
'--pc-box-margin-top': marginTop ? `var(--p-space-${marginTop})` : '',
121-
'--pc-box-padding': padding ? `var(--p-space-${padding})` : '',
122-
'--pc-box-padding-bottom': paddingBottom
123-
? `var(--p-space-${paddingBottom})`
147+
'--pc-box-padding-left': paddings.left
148+
? `var(--p-space-${paddings.left})`
124149
: '',
125-
'--pc-box-padding-left': paddingLeft
126-
? `var(--p-space-${paddingLeft})`
150+
'--pc-box-padding-right': paddings.right
151+
? `var(--p-space-${paddings.right})`
127152
: '',
128-
'--pc-box-padding-right': paddingRight
129-
? `var(--p-space-${paddingRight})`
153+
'--pc-box-padding-top': paddings.top
154+
? `var(--p-space-${paddings.top})`
130155
: '',
131-
'--pc-box-padding-top': paddingTop ? `var(--p-space-${paddingTop})` : '',
132156
} as React.CSSProperties;
133157

134158
const className = classNames(

0 commit comments

Comments
 (0)