Skip to content

[Box] Add support for responsive padding #7697

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

Merged
merged 11 commits into from
Nov 16, 2022
Merged

[Box] Add support for responsive padding #7697

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
6885006
Add responsive-props mixin
aveline Nov 10, 2022
5411b3c
Add responsive padding to `Box`
aveline Nov 10, 2022
23d41c3
changeset
aveline Nov 10, 2022
ceb42d3
👕 linting
aveline Nov 10, 2022
5dae27a
👕
aveline Nov 14, 2022
11f4616
👕
aveline Nov 14, 2022
0bd5c77
💚
aveline Nov 14, 2022
b01f4d9
Fix responsive css mixin
aveline Nov 14, 2022
955a76c
👕
aveline Nov 14, 2022
7a72e21
👕
aveline Nov 14, 2022
301dc4b
Update polaris-react/src/utilities/css.ts
aveline Nov 15, 2022
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/small-gifts-hope.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': minor
---

Added support for responsive padding to `Box`
35 changes: 28 additions & 7 deletions polaris-react/src/components/Box/Box.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,32 @@
@import '../../styles/common';

.Box {
@include responsive-props('box', 'padding', 'padding');
@include responsive-props(
'box',
'padding-block-end',
'padding-block-end',
'padding'
);
@include responsive-props(
'box',
'padding-block-start',
'padding-block-start',
'padding'
);
@include responsive-props(
'box',
'padding-inline-start',
'padding-inline-start',
'padding'
);
@include responsive-props(
'box',
'padding-inline-end',
'padding-inline-end',
'padding'
);
Comment on lines +4 to +28
Copy link
Member

Choose a reason for hiding this comment

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

This responsive-props abstraction looks great and really cleans up duplications. However, I think there may be an opportunity to further optimize the outputted CSS. Right now, the generated CSS creates 4 media queries for each mixin usage which the Sass compiler can't deduplicate. I don't think we need to address the optimization in this PR, but it may be worth investigating an API where you define all responsive props upfront and inject them into only 4 media queries e.g.

@include responsive-props(
  ('box', 'padding', 'padding'),
  ('box', 'padding-block-start', 'padding-block-start', 'padding'),
  ('box', 'padding-block-end', 'padding-block-start', 'padding'),
  // etc.
);

Copy link
Member

@aaronccasanova aaronccasanova Nov 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Couldn't shake it! Here's a proof of concept that consolidates the outputted CSS:

Updated responsive-props API 
$p-breakpoints-sm-up: '(min-width: 1px)';
$p-breakpoints-md-up: '(min-width: 2px)';
$p-breakpoints-lg-up: '(min-width: 3px)';
$p-breakpoints-xl-up: '(min-width: 4px)';

@mixin responsive-props($props...) {
  @each $prop in $props {
    $componentName: map-get($prop, component-name);
    $componentProp: map-get($prop, component-prop);

    --pc-#{$componentName}-#{$componentProp}-xs: initial;

    --pc-#{$componentName}-#{$componentProp}-sm:
      var(--pc-#{$componentName}-#{$componentProp}-xs);

    --pc-#{$componentName}-#{$componentProp}-md: var(
      --pc-#{$componentName}-#{$componentProp}-sm
    );
    
    --pc-#{$componentName}-#{$componentProp}-lg: var(
      --pc-#{$componentName}-#{$componentProp}-md
    );
    
    --pc-#{$componentName}-#{$componentProp}-xl: var(
      --pc-#{$componentName}-#{$componentProp}-lg
    );
  }
  
  @each $prop in $props {
    $componentName: map-get($prop, component-name);
    $componentProp: map-get($prop, component-prop);
    $declarationProp: map-get($prop, declaration-prop);
    $fallbackProp: map-get($prop, fallback-prop);

    #{$declarationProp}: var(
      --pc-#{$componentName}-#{$componentProp}-xs,
      if($fallbackProp, --pc-#{$componentName}-#{$fallbackProp}-xs, null),
     );
  }

  @media #{$p-breakpoints-sm-up} {
    @each $config in $props {
      $componentName: map-get($config, component-name);
      $componentProp: map-get($config, component-prop);
      $declarationProp: map-get($config, declaration-prop);
      $fallbackProp: map-get($config, fallback-prop);

      #{$declarationProp}: var(
        --pc-#{$componentName}-#{$componentProp}-sm,
        if($fallbackProp, --pc-#{$componentName}-#{$fallbackProp}-sm, null),
      );
    }
  }

  @media #{$p-breakpoints-md-up} {
    @each $prop in $props {
      $componentName: map-get($prop, component-name);
      $componentProp: map-get($prop, component-prop);
      $declarationProp: map-get($prop, declaration-prop);
      $fallbackProp: map-get($prop, fallback-prop);

      #{$declarationProp}: var(
        --pc-#{$componentName}-#{$componentProp}-md,
        if($fallbackProp, --pc-#{$componentName}-#{$fallbackProp}-md, null),
      );
    }
  }

  @media #{$p-breakpoints-lg-up} {
    @each $prop in $props {
      $componentName: map-get($prop, component-name);
      $componentProp: map-get($prop, component-prop);
      $declarationProp: map-get($prop, declaration-prop);
      $fallbackProp: map-get($prop, fallback-prop);

      #{$declarationProp}: var(
        --pc-#{$componentName}-#{$componentProp}-lg,
        if($fallbackProp, --pc-#{$componentName}-#{$fallbackProp}-lg, null),
      );
    }
  }

  @media #{$p-breakpoints-xl-up} {
    @each $prop in $props {
      $componentName: map-get($prop, component-name);
      $componentProp: map-get($prop, component-prop);
      $declarationProp: map-get($prop, declaration-prop);
      $fallbackProp: map-get($prop, fallback-prop);

      #{$declarationProp}: var(
        --pc-#{$componentName}-#{$componentProp}-xl,
        if($fallbackProp, --pc-#{$componentName}-#{$fallbackProp}-xl, null),
      );
    }
  }
}

Example usage:

.Box {
  @include responsive-props(
    (
      component-name: box,
      component-prop: padding,
      declaration-prop: padding,
    ),
    (
      component-name: box,
      component-prop: padding-block-start,
      declaration-prop: padding-block-start,
      fallback-prop: padding,
    ),
  );
}
Example output: 
.Box {
  --pc-box-padding-xs: initial;
  --pc-box-padding-sm:
    var(--pc-box-padding-xs);
  --pc-box-padding-md: var(
    --pc-box-padding-sm
  );
  --pc-box-padding-lg: var(
    --pc-box-padding-md
  );
  --pc-box-padding-xl: var(
    --pc-box-padding-lg
  );
  --pc-box-padding-block-start-xs: initial;
  --pc-box-padding-block-start-sm:
    var(--pc-box-padding-block-start-xs);
  --pc-box-padding-block-start-md: var(
    --pc-box-padding-block-start-sm
  );
  --pc-box-padding-block-start-lg: var(
    --pc-box-padding-block-start-md
  );
  --pc-box-padding-block-start-xl: var(
    --pc-box-padding-block-start-lg
  );
  padding: var(--pc-box-padding-xs, );
  padding-block-start: var(--pc-box-padding-block-start-xs, --pc-box-padding-xs);
}
@media (min-width: 1px) {
  .Box {
    padding: var(--pc-box-padding-sm, );
    padding-block-start: var(--pc-box-padding-block-start-sm, --pc-box-padding-sm);
  }
}
@media (min-width: 2px) {
  .Box {
    padding: var(--pc-box-padding-md, );
    padding-block-start: var(--pc-box-padding-block-start-md, --pc-box-padding-md);
  }
}
@media (min-width: 3px) {
  .Box {
    padding: var(--pc-box-padding-lg, );
    padding-block-start: var(--pc-box-padding-block-start-lg, --pc-box-padding-lg);
  }
}
@media (min-width: 4px) {
  .Box {
    padding: var(--pc-box-padding-xl, );
    padding-block-start: var(--pc-box-padding-block-start-xl, --pc-box-padding-xl);
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is great and will look at this in a next iteration. Going to merge this as is now to unblock some other work.


--pc-box-shadow: initial;
--pc-box-background: initial;
--pc-box-border-radius: initial;
Expand Down Expand Up @@ -69,13 +97,6 @@
max-width: var(--pc-box-max-width);
overflow-x: var(--pc-box-overflow-x);
overflow-y: var(--pc-box-overflow-y);
padding-block-end: var(--pc-box-padding-block-end, var(--pc-box-padding));
padding-inline-start: var(
--pc-box-padding-inline-start,
var(--pc-box-padding)
);
padding-inline-end: var(--pc-box-padding-inline-end, var(--pc-box-padding));
padding-block-start: var(--pc-box-padding-block-start, var(--pc-box-padding));
width: var(--pc-box-width);
-webkit-overflow-scrolling: touch;
}
43 changes: 42 additions & 1 deletion polaris-react/src/components/Box/Box.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import type {ComponentMeta} from '@storybook/react';
import {Box, Icon} from '@shopify/polaris';
import {AlphaStack, Box, Icon} from '@shopify/polaris';
import {PaintBrushMajor} from '@shopify/polaris-icons';

export default {
Expand Down Expand Up @@ -30,3 +30,44 @@ export function BoxWithBorderRadius() {
</Box>
);
}

export function BoxWithResponsivePadding() {
return (
<AlphaStack>
<Box background="surface" padding={{xs: '2', sm: '8'}} border="dark">
<Icon source={PaintBrushMajor} color="base" />
</Box>
<Box
background="surface"
padding="2"
paddingBlockStart={{xs: '4', sm: '10'}}
border="dark"
>
<Icon source={PaintBrushMajor} color="base" />
</Box>
<Box
background="surface"
padding="2"
paddingBlockEnd={{xs: '4', sm: '10'}}
border="dark"
>
<Icon source={PaintBrushMajor} color="base" />
</Box>
<Box
background="surface"
padding="2"
paddingInlineStart={{xs: '4', sm: '10'}}
border="dark"
>
<Icon source={PaintBrushMajor} color="base" />
</Box>
<Box
background="surface"
paddingInlineEnd={{xs: '4', sm: '10'}}
border="dark"
>
<Icon source={PaintBrushMajor} color="base" />
</Box>
</AlphaStack>
);
}
71 changes: 38 additions & 33 deletions polaris-react/src/components/Box/Box.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ import type {
SpacingSpaceScale,
} from '@shopify/polaris-tokens';

import {classNames, sanitizeCustomProperties} from '../../utilities/css';
import {
getResponsiveProps,
ResponsiveProp,
classNames,
sanitizeCustomProperties,
} from '../../utilities/css';

import styles from './Box.scss';

Expand Down Expand Up @@ -42,6 +47,8 @@ export type BorderTokenAlias =
| 'divider-on-dark'
| 'transparent';

type Spacing = ResponsiveProp<SpacingSpaceScale>;

interface Border {
blockStart: BorderTokenAlias;
blockEnd: BorderTokenAlias;
Expand Down Expand Up @@ -75,13 +82,6 @@ interface BorderRadius {
endEnd: BorderRadiusTokenScale;
}

interface Spacing {
blockStart: SpacingSpaceScale;
blockEnd: SpacingSpaceScale;
inlineStart: SpacingSpaceScale;
inlineEnd: SpacingSpaceScale;
}

interface BorderWidth {
blockStart: ShapeBorderWidthScale;
blockEnd: ShapeBorderWidthScale;
Expand Down Expand Up @@ -139,15 +139,15 @@ export interface BoxProps {
/** Clip vertical content of children */
overflowY?: Overflow;
/** Spacing around children */
padding?: SpacingSpaceScale;
padding?: Spacing;
/** Vertical start spacing around children */
paddingBlockStart?: SpacingSpaceScale;
paddingBlockStart?: Spacing;
/** Vertical end spacing around children */
paddingBlockEnd?: SpacingSpaceScale;
paddingBlockEnd?: Spacing;
/** Horizontal start spacing around children */
paddingInlineStart?: SpacingSpaceScale;
paddingInlineStart?: Spacing;
/** Horizontal end spacing around children */
paddingInlineEnd?: SpacingSpaceScale;
paddingInlineEnd?: Spacing;
/** Shadow */
shadow?: DepthShadowAlias;
/** Set width of container */
Expand Down Expand Up @@ -215,13 +215,6 @@ export const Box = forwardRef<HTMLElement, BoxProps>(
inlineEnd: borderInlineEndWidth,
} as BorderWidth;

const paddings = {
blockEnd: paddingBlockEnd,
inlineStart: paddingInlineStart,
inlineEnd: paddingInlineEnd,
blockStart: paddingBlockStart,
} as Spacing;

const style = {
'--pc-box-color': color ? `var(--p-${color})` : undefined,
'--pc-box-background': background ? `var(--p-${background})` : undefined,
Expand Down Expand Up @@ -273,19 +266,31 @@ export const Box = forwardRef<HTMLElement, BoxProps>(
'--pc-box-max-width': maxWidth,
'--pc-box-overflow-x': overflowX,
'--pc-box-overflow-y': overflowY,
'--pc-box-padding': padding ? `var(--p-space-${padding})` : undefined,
'--pc-box-padding-block-end': paddings.blockEnd
? `var(--p-space-${paddings.blockEnd})`
: undefined,
'--pc-box-padding-inline-start': paddings.inlineStart
? `var(--p-space-${paddings.inlineStart})`
: undefined,
'--pc-box-padding-inline-end': paddings.inlineEnd
? `var(--p-space-${paddings.inlineEnd})`
: undefined,
'--pc-box-padding-block-start': paddings.blockStart
? `var(--p-space-${paddings.blockStart})`
: undefined,
...getResponsiveProps('box', 'padding', 'space', padding),
...getResponsiveProps(
'box',
'padding-block-end',
'space',
paddingBlockEnd,
),
...getResponsiveProps(
'box',
'padding-block-start',
'space',
paddingBlockStart,
),
...getResponsiveProps(
'box',
'padding-inline-start',
'space',
paddingInlineStart,
),
...getResponsiveProps(
'box',
'padding-inline-end',
'space',
paddingInlineEnd,
),
'--pc-box-shadow': shadow ? `var(--p-shadow-${shadow})` : undefined,
'--pc-box-width': width,
} as React.CSSProperties;
Expand Down
19 changes: 16 additions & 3 deletions polaris-react/src/components/Box/tests/Box.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe('Box', () => {

expect(box).toContainReactComponent('div', {
style: {
'--pc-box-padding-inline-start': 'var(--p-space-2)',
'--pc-box-padding-inline-start-xs': 'var(--p-space-2)',
} as React.CSSProperties,
});
});
Expand All @@ -93,9 +93,22 @@ describe('Box', () => {

expect(box).toContainReactComponent('div', {
style: {
'--pc-box-padding': 'var(--p-space-1)',
'--pc-box-padding-inline-start': 'var(--p-space-2)',
'--pc-box-padding-xs': 'var(--p-space-1)',
'--pc-box-padding-inline-start-xs': 'var(--p-space-2)',
} as React.CSSProperties,
});
});

it('accepts padding based on breakpoints', () => {
const box = mountWithApp(
<Box padding={{xs: '2', md: '8'}}>{children}</Box>,
);

expect(box).toContainReactComponent('div', {
style: expect.objectContaining({
'--pc-box-padding-md': 'var(--p-space-8)',
'--pc-box-padding-xs': 'var(--p-space-2)',
}) as React.CSSProperties,
});
});
});
1 change: 1 addition & 0 deletions polaris-react/src/styles/_common.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
@import './shared/icons';
@import './shared/layout';
@import './shared/page';
@import './shared/responsive-props';
@import './shared/typography';
@import './shared/interaction-state';

Expand Down
72 changes: 72 additions & 0 deletions polaris-react/src/styles/shared/_responsive-props.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
@mixin responsive-props(
$componentName,
$componentProp,
$declartionProp,
$shorthandFallback: null
) {
--pc-#{$componentName}-#{$componentProp}-xs: initial;
--pc-#{$componentName}-#{$componentProp}-sm: var(
--pc-#{$componentName}-#{$componentProp}-xs
);
--pc-#{$componentName}-#{$componentProp}-md: var(
--pc-#{$componentName}-#{$componentProp}-sm
);
--pc-#{$componentName}-#{$componentProp}-lg: var(
--pc-#{$componentName}-#{$componentProp}-md
);
--pc-#{$componentName}-#{$componentProp}-xl: var(
--pc-#{$componentName}-#{$componentProp}-lg
);
@if $shorthandFallback {
#{$declartionProp}: var(
--pc-#{$componentName}-#{$componentProp}-xs,
var(--pc-#{$componentName}-#{$shorthandFallback}-xs)
);

@media #{$p-breakpoints-sm-up} {
#{$declartionProp}: var(
--pc-#{$componentName}-#{$componentProp}-sm,
var(--pc-#{$componentName}-#{$shorthandFallback}-sm)
);
}

@media #{$p-breakpoints-md-up} {
#{$declartionProp}: var(
--pc-#{$componentName}-#{$componentProp}-md,
var(--pc-#{$componentName}-#{$shorthandFallback}-md)
);
}

@media #{$p-breakpoints-lg-up} {
#{$declartionProp}: var(
--pc-#{$componentName}-#{$componentProp}-lg,
var(--pc-#{$componentName}-#{$shorthandFallback}-lg)
);
}

@media #{$p-breakpoints-xl-up} {
#{$declartionProp}: var(
--pc-#{$componentName}-#{$componentProp}-xl,
var(--pc-#{$componentName}-#{$shorthandFallback}-xl)
);
}
} @else {
#{$declartionProp}: var(--pc-#{$componentName}-#{$componentProp}-xs);

@media #{$p-breakpoints-sm-up} {
#{$declartionProp}: var(--pc-#{$componentName}-#{$componentProp}-sm);
}

@media #{$p-breakpoints-md-up} {
#{$declartionProp}: var(--pc-#{$componentName}-#{$componentProp}-md);
}

@media #{$p-breakpoints-lg-up} {
#{$declartionProp}: var(--pc-#{$componentName}-#{$componentProp}-lg);
}

@media #{$p-breakpoints-xl-up} {
#{$declartionProp}: var(--pc-#{$componentName}-#{$componentProp}-xl);
}
}
}
4 changes: 3 additions & 1 deletion polaris-react/src/utilities/css.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,14 @@ export function getResponsiveProps(
componentName: string,
componentProp: string,
tokenSubgroup: string,
responsiveProp:
responsiveProp?:
| string
| {
[Breakpoint in BreakpointsAlias]?: string;
},
) {
if (!responsiveProp) return {};

if (typeof responsiveProp === 'string') {
return {
[`--pc-${componentName}-${componentProp}-xs`]: `var(--p-${tokenSubgroup}-${responsiveProp})`,
Expand Down