Skip to content

[Box] Add position and visibility properties #7755

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 6 commits into from
Nov 18, 2022
Merged

[Box] Add position and visibility properties #7755

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
41bd9b9
Add position, z-index, opacity, visibility props to Box
kyledurand Nov 17, 2022
958ebc8
changeset
kyledurand Nov 17, 2022
93ac7e5
use spacing values on top right bottom left
kyledurand Nov 17, 2022
36c9bdb
Create type for position
kyledurand Nov 17, 2022
c48a12b
re-order properties
kyledurand Nov 17, 2022
5a1b5b2
Document props
kyledurand Nov 18, 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/gorgeous-ants-hammer.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Added position, opacity, top, left, right, bottom, z-index to `Box`
12 changes: 12 additions & 0 deletions polaris-react/src/components/Box/Box.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@
--pc-box-padding-block-start: initial;
--pc-box-width: initial;
--pc-box-border-width: var(--p-border-width-1);
--pc-box-top: initial;
--pc-box-right: initial;
--pc-box-bottom: initial;
--pc-box-left: initial;
top: var(--pc-box-top);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should rename this to insetBlockStart to be consistent with the other properties where we have started to use the logical properties naming. Same with insetBlockEnd, insetInlineStart and insetInlineEnd.

Copy link
Contributor

Choose a reason for hiding this comment

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

Issue to update: #7771

right: var(--pc-box-right);
bottom: var(--pc-box-bottom);
left: var(--pc-box-left);
background-color: var(--pc-box-background);
box-shadow: var(--pc-box-shadow);
border-radius: var(--pc-box-border-radius);
Expand Down Expand Up @@ -100,3 +108,7 @@
width: var(--pc-box-width);
-webkit-overflow-scrolling: touch;
}

.visuallyHidden {
@include visually-hidden;
}
137 changes: 65 additions & 72 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 @@ -20,8 +20,8 @@ import {
import styles from './Box.scss';

type Element = 'div' | 'span' | 'section';

type Overflow = 'hidden' | 'scroll';
type Position = 'relative' | 'absolute' | 'fixed' | 'sticky';

export type ColorTokenScale =
| 'text'
Expand Down Expand Up @@ -49,13 +49,6 @@ export type BorderTokenAlias =

type Spacing = ResponsiveProp<SpacingSpaceScale>;

interface Border {
blockStart: BorderTokenAlias;
blockEnd: BorderTokenAlias;
inlineStart: BorderTokenAlias;
inlineEnd: BorderTokenAlias;
}

export type BorderRadiusTokenScale =
| '05'
| '1'
Expand All @@ -75,20 +68,6 @@ export type BackgroundColors =
| ColorsActionTokenAlias
| ColorsSurfaceTokenAlias;

interface BorderRadius {
startStart: BorderRadiusTokenScale;
startEnd: BorderRadiusTokenScale;
endStart: BorderRadiusTokenScale;
endEnd: BorderRadiusTokenScale;
}

interface BorderWidth {
blockStart: ShapeBorderWidthScale;
blockEnd: ShapeBorderWidthScale;
inlineStart: ShapeBorderWidthScale;
inlineEnd: ShapeBorderWidthScale;
}

export interface BoxProps {
/** HTML Element type */
as?: Element;
Expand Down Expand Up @@ -128,11 +107,11 @@ export interface BoxProps {
color?: ColorTokenScale;
/** HTML id attribute */
id?: string;
/** Set minimum height of container */
/** Minimum height of container */
minHeight?: string;
/** Set minimum width of container */
/** Minimum width of container */
minWidth?: string;
/** Set maximum width of container */
/** Maximum width of container */
maxWidth?: string;
/** Clip horizontal content of children */
overflowX?: Overflow;
Expand Down Expand Up @@ -170,10 +149,27 @@ export interface BoxProps {
paddingInlineEnd?: Spacing;
/** Shadow */
shadow?: DepthShadowAlias;
/** Set width of container */
/** Width of container */
width?: string;
/** Elements to display inside box */
children?: React.ReactNode;
// These could be moved to new layout component(s) in the future
/** Position of the box */
position?: Position;
/** Top position of the box */
top?: Spacing;
/** Bottom position of the box */
right?: Spacing;
/** Left position of the box */
bottom?: Spacing;
/** Right position of the box */
left?: Spacing;
/** Opcity of the box */
opacity?: string;
/** Visually hide the contents (still announced by screenreader) */
visuallyHidden?: boolean;
/** z-index of the box */
zIndex?: string;
}

export const Box = forwardRef<HTMLElement, BoxProps>(
Expand Down Expand Up @@ -211,75 +207,62 @@ export const Box = forwardRef<HTMLElement, BoxProps>(
paddingInlineEnd,
shadow,
width,
visuallyHidden,
position,
top,
right,
bottom,
left,
zIndex,
opacity,
},
ref,
) => {
const borders = {
blockEnd: borderBlockEnd,
inlineStart: borderInlineStart,
inlineEnd: borderInlineEnd,
blockStart: borderBlockStart,
} as Border;

const borderRadiuses = {
endStart: borderRadiusEndStart,
endEnd: borderRadiusEndEnd,
startStart: borderRadiusStartStart,
startEnd: borderRadiusStartEnd,
} as BorderRadius;

const borderWidths = {
blockStart: borderBlockStartWidth,
blockEnd: borderBlockEndWidth,
inlineStart: borderInlineStartWidth,
inlineEnd: borderInlineEndWidth,
} as BorderWidth;
Comment on lines -217 to -236
Copy link
Member Author

Choose a reason for hiding this comment

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

Any reason why we were remapping these? Just seemed to add complexity to me

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I agree. I actually removed the padding one when I added responsive padding to Box. I think it would be fine to remove these as well.


const style = {
'--pc-box-color': color ? `var(--p-${color})` : undefined,
'--pc-box-background': background ? `var(--p-${background})` : undefined,
'--pc-box-border': border ? `var(--p-border-${border})` : undefined,
'--pc-box-border-block-end': borders.blockEnd
? `var(--p-border-${borders.blockEnd})`
'--pc-box-border-block-end': borderBlockEnd
? `var(--p-border-${borderBlockEnd})`
: undefined,
'--pc-box-border-inline-start': borders.inlineStart
? `var(--p-border-${borders.inlineStart})`
'--pc-box-border-inline-start': borderInlineStart
? `var(--p-border-${borderInlineStart})`
: undefined,
'--pc-box-border-inline-end': borders.inlineEnd
? `var(--p-border-${borders.inlineEnd})`
'--pc-box-border-inline-end': borderInlineEnd
? `var(--p-border-${borderInlineEnd})`
: undefined,
'--pc-box-border-block-start': borders.blockStart
? `var(--p-border-${borders.blockStart})`
'--pc-box-border-block-start': borderBlockStart
? `var(--p-border-${borderBlockStart})`
: undefined,
'--pc-box-border-radius': borderRadius
? `var(--p-border-radius-${borderRadius})`
: undefined,
'--pc-box-border-radius-end-start': borderRadiuses.endStart
? `var(--p-border-radius-${borderRadiuses.endStart})`
'--pc-box-border-radius-end-start': borderRadiusEndStart
? `var(--p-border-radius-${borderRadiusEndStart})`
: undefined,
'--pc-box-border-radius-end-end': borderRadiuses.endEnd
? `var(--p-border-radius-${borderRadiuses.endEnd})`
'--pc-box-border-radius-end-end': borderRadiusEndEnd
? `var(--p-border-radius-${borderRadiusEndEnd})`
: undefined,
'--pc-box-border-radius-start-start': borderRadiuses.startStart
? `var(--p-border-radius-${borderRadiuses.startStart})`
'--pc-box-border-radius-start-start': borderRadiusStartStart
? `var(--p-border-radius-${borderRadiusStartStart})`
: undefined,
'--pc-box-border-radius-start-end': borderRadiuses.startEnd
? `var(--p-border-radius-${borderRadiuses.startEnd})`
'--pc-box-border-radius-start-end': borderRadiusStartEnd
? `var(--p-border-radius-${borderRadiusStartEnd})`
: undefined,
'--pc-box-border-width': borderWidth
? `var(--p-border-width-${borderWidth})`
: undefined,
'--pc-box-border-block-start-width': borderWidths.blockStart
? `var(--p-border-width-${borderWidths.blockStart})`
'--pc-box-border-block-start-width': borderBlockStartWidth
? `var(--p-border-width-${borderBlockStartWidth})`
: undefined,
'--pc-box-border-block-end-width': borderWidths.blockEnd
? `var(--p-border-width-${borderWidths.blockEnd})`
'--pc-box-border-block-end-width': borderBlockEndWidth
? `var(--p-border-width-${borderBlockEndWidth})`
: undefined,
'--pc-box-border-inline-start-width': borderWidths.inlineStart
? `var(--p-border-width-${borderWidths.inlineStart})`
'--pc-box-border-inline-start-width': borderInlineStartWidth
? `var(--p-border-width-${borderInlineStartWidth})`
: undefined,
'--pc-box-border-inline-end-width': borderWidths.inlineEnd
? `var(--p-border-width-${borderWidths.inlineEnd})`
'--pc-box-border-inline-end-width': borderInlineEndWidth
? `var(--p-border-width-${borderInlineEndWidth})`
: undefined,
'--pc-box-min-height': minHeight,
'--pc-box-min-width': minWidth,
Expand Down Expand Up @@ -313,9 +296,19 @@ export const Box = forwardRef<HTMLElement, BoxProps>(
),
'--pc-box-shadow': shadow ? `var(--p-shadow-${shadow})` : undefined,
'--pc-box-width': width,
position,
'--pc-box-top': top ? `var(--p-space-${top})` : undefined,
'--pc-box-right': right ? `var(--p-space-${right})` : undefined,
'--pc-box-bottom': bottom ? `var(--p-space-${bottom})` : undefined,
'--pc-box-left': left ? `var(--p-space-${left})` : undefined,
zIndex,
opacity,
} as React.CSSProperties;

const className = classNames(styles.Box);
const className = classNames(
styles.Box,
visuallyHidden && styles.visuallyHidden,
);

return createElement(
as,
Expand Down
Loading