feat: add should forward props to box and text (#2285)

* feat: add should forward props to box and text * fix: types and changesets * fix: exclude size from being prevented * fix: modal overlay to use styled component * chore: changeset Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
twilio-labs · Mar 25, 2022 · cf5878d · cf5878d
1 parent 003ae44
commit cf5878d
Show file tree

Hide file tree

Showing 27 changed files with 251 additions and 281 deletions.
diff --git a/.changeset/red-masks-obey.md b/.changeset/red-masks-obey.md
@@ -0,0 +1,6 @@
+---
+'@twilio-paste/modal-dialog-primitive': minor
+'@twilio-paste/core': minor
+---
+
+[Modal primitive] update to Reach Modal version number to v0.16.2
diff --git a/.changeset/shaggy-weeks-return.md b/.changeset/shaggy-weeks-return.md
@@ -0,0 +1,6 @@
+---
+'@twilio-paste/modal': patch
+'@twilio-paste/core': patch
+---
+
+[Modal] Switch modal overlay to using a pure styled component and not Box
diff --git a/.changeset/spotty-lobsters-impress.md b/.changeset/spotty-lobsters-impress.md
@@ -0,0 +1,6 @@
+---
+'@twilio-paste/styling-library': minor
+'@twilio-paste/core': minor
+---
+
+[Styling library] two new exports from styled system, `props` and `createShouldForwardProp`.
diff --git a/.changeset/wicked-vans-add.md b/.changeset/wicked-vans-add.md
@@ -0,0 +1,7 @@
+---
+'@twilio-paste/box': minor
+'@twilio-paste/text': minor
+'@twilio-paste/core': minor
+---
+
+[Box, Text] Prevented styling props from being forwarded to their HTML DOM nodes in the browser for a cleaner DOM inspector view
diff --git a/@types/@styled-system/index.d.ts b/@types/@styled-system/index.d.ts
diff --git a/@types/@styled-system/should-forward-prop/index.d.ts b/@types/@styled-system/should-forward-prop/index.d.ts
diff --git a/@types/@styled-system/theme-get/index.d.ts b/@types/@styled-system/theme-get/index.d.ts
diff --git a/packages/paste-core/components/avatar/__tests__/__snapshots__/avatar.test.tsx.snap b/packages/paste-core/components/avatar/__tests__/__snapshots__/avatar.test.tsx.snap
@@ -49,9 +49,7 @@ exports[`Avatar image should render responsive css with an image 1`] = `
 
 <div
     class="emotion-1"
-    color="colorText"
     data-paste-element="AVATAR"
-    overflow="hidden"
   >
     <img
       alt="avatar example"
@@ -119,18 +117,11 @@ exports[`Avatar intials should render responsive css 1`] = `
 
 <div
     class="emotion-1"
-    color="colorText"
     data-paste-element="AVATAR"
-    overflow="hidden"
   >
     <abbr
       class="emotion-0"
-      color="inherit"
       data-paste-element="TEXT"
-      display="block"
-      font-size="fontSize10,fontSize10,fontSize60"
-      font-weight="fontWeightBold"
-      text-decoration="none"
       title="Simon Taggart"
     >
       ST

diff --git a/packages/paste-core/components/button/__tests__/button.test.tsx b/packages/paste-core/components/button/__tests__/button.test.tsx
@@ -627,7 +627,7 @@ describe('Button', () => {
 
       expect(getByTestId('loading-link-styles')).toHaveStyleRule('text-align', 'left');
 
-      expect(buttonContent).toHaveAttribute('opacity', '0');
+      expect(buttonContent).toHaveStyleRule('opacity', '0');
 
       expect(loadingIconWrapper).toHaveStyleRule('position', 'absolute');
       expect(loadingIconWrapper).toHaveStyleRule('top', '0');

diff --git a/packages/paste-core/components/combobox/__tests__/virtualization.spec.tsx b/packages/paste-core/components/combobox/__tests__/virtualization.spec.tsx
@@ -59,23 +59,23 @@ describe('Combobox - Virtualization', () => {
 
       const [first, second, third, fourth, fifth] = optionElements;
 
-      expect(first).toHaveAttribute('transform', 'translateY(8px)');
+      expect(first).toHaveStyleRule('transform', 'translateY(8px)');
       expect(first).toHaveAttribute('aria-setsize', '5');
       expect(first).toHaveAttribute('aria-posinset', '1');
 
-      expect(second).toHaveAttribute('transform', 'translateY(44px)');
+      expect(second).toHaveStyleRule('transform', 'translateY(44px)');
       expect(second).toHaveAttribute('aria-setsize', '5');
       expect(second).toHaveAttribute('aria-posinset', '2');
 
-      expect(third).toHaveAttribute('transform', 'translateY(80px)');
+      expect(third).toHaveStyleRule('transform', 'translateY(80px)');
       expect(third).toHaveAttribute('aria-setsize', '5');
       expect(third).toHaveAttribute('aria-posinset', '3');
 
-      expect(fourth).toHaveAttribute('transform', 'translateY(116px)');
+      expect(fourth).toHaveStyleRule('transform', 'translateY(116px)');
       expect(fourth).toHaveAttribute('aria-setsize', '5');
       expect(fourth).toHaveAttribute('aria-posinset', '4');
 
-      expect(fifth).toHaveAttribute('transform', 'translateY(152px)');
+      expect(fifth).toHaveStyleRule('transform', 'translateY(152px)');
       expect(fifth).toHaveAttribute('aria-setsize', '5');
       expect(fifth).toHaveAttribute('aria-posinset', '5');
     });
@@ -151,23 +151,23 @@ describe('Combobox - Virtualization', () => {
 
       const [first, second, third, fourth, fifth] = optionElements;
 
-      expect(first).toHaveAttribute('transform', 'translateY(8px)');
+      expect(first).toHaveStyleRule('transform', 'translateY(8px)');
       expect(first).toHaveAttribute('aria-setsize', '5');
       expect(first).toHaveAttribute('aria-posinset', '1');
 
-      expect(second).toHaveAttribute('transform', 'translateY(44px)');
+      expect(second).toHaveStyleRule('transform', 'translateY(44px)');
       expect(second).toHaveAttribute('aria-setsize', '5');
       expect(second).toHaveAttribute('aria-posinset', '2');
 
-      expect(third).toHaveAttribute('transform', 'translateY(80px)');
+      expect(third).toHaveStyleRule('transform', 'translateY(80px)');
       expect(third).toHaveAttribute('aria-setsize', '5');
       expect(third).toHaveAttribute('aria-posinset', '3');
 
-      expect(fourth).toHaveAttribute('transform', 'translateY(116px)');
+      expect(fourth).toHaveStyleRule('transform', 'translateY(116px)');
       expect(fourth).toHaveAttribute('aria-setsize', '5');
       expect(fourth).toHaveAttribute('aria-posinset', '4');
 
-      expect(fifth).toHaveAttribute('transform', 'translateY(152px)');
+      expect(fifth).toHaveStyleRule('transform', 'translateY(152px)');
       expect(fifth).toHaveAttribute('aria-setsize', '5');
       expect(fifth).toHaveAttribute('aria-posinset', '5');
     });

diff --git a/packages/paste-core/components/input/__tests__/input.test.tsx b/packages/paste-core/components/input/__tests__/input.test.tsx
@@ -159,11 +159,11 @@ describe('Input block props', () => {
   };
 
   it('should not pass width and classname props', () => {
-    // @ts-ignore this is on purpose
-    const {getByRole} = render(<Input {...initialProps} />);
-    const RenderedInput = getByRole('textbox');
+    // @ts-expect-error we are passing unexpected props on purpose
+    render(<Input {...initialProps} />);
+    const RenderedInput = screen.getByRole('textbox');
 
-    expect(RenderedInput.getAttribute('width')).toEqual('100%');
+    expect(RenderedInput).toHaveStyleRule('width', '100%');
 
     const classNames = RenderedInput.getAttribute('class');
     expect(classNames).toBeDefined();

diff --git a/packages/paste-core/components/modal/src/Modal.tsx b/packages/paste-core/components/modal/src/Modal.tsx
@@ -2,16 +2,19 @@ import * as React from 'react';
 import * as PropTypes from 'prop-types';
 import {css, styled} from '@twilio-paste/styling-library';
 import {useTransition, animated} from '@twilio-paste/animation-library';
-import {safelySpreadBoxProps, Box} from '@twilio-paste/box';
+import {safelySpreadBoxProps, Box, getCustomElementStyles} from '@twilio-paste/box';
 import type {BoxElementProps} from '@twilio-paste/box';
 import {pasteBaseStyles} from '@twilio-paste/theme';
 import {ModalDialogPrimitiveOverlay, ModalDialogPrimitiveContent} from '@twilio-paste/modal-dialog-primitive';
 import {ModalContext} from './ModalContext';
 import {addConsoleHeightPatch, removeConsoleHeightPatch} from './utils/consoleUtils';
 
+type Sizes = 'default' | 'wide';
+
 export const ModalDialogOverlay = animated(
   /* eslint-disable emotion/syntax-preference */
-  styled(ModalDialogPrimitiveOverlay)(
+  // @ts-ignore Just like in box I can't work out how to stop the styled div color prop from emotion clashing with our color style prop in BoxProps
+  styled(ModalDialogPrimitiveOverlay)<{variant?: Sizes}>(
     css({
       position: 'fixed',
       top: 0,
@@ -31,13 +34,12 @@ export const ModalDialogOverlay = animated(
     // some of the base styles that we rely on inheriting from
     // such as font-family and line-height so that compositions
     // of paste components in the modal are styled correctly
-    pasteBaseStyles
+    pasteBaseStyles,
+    getCustomElementStyles
   )
   /* eslint-enable */
 );
 
-type Sizes = 'default' | 'wide';
-
 interface ModalDialogContentProps {
   size?: Sizes;
   children: React.ReactNode;
@@ -129,14 +131,12 @@ const Modal = React.forwardRef<HTMLDivElement, ModalProps>(
         {transitions(
           (styles, item) =>
             item && (
-              <Box
-                // @ts-expect-error Render overlay as box for customization
-                as={ModalDialogOverlay}
+              <ModalDialogOverlay
                 onDismiss={onDismiss}
                 allowPinchZoom={allowPinchZoom}
                 initialFocusRef={initialFocusRef}
                 style={{opacity: styles.opacity}}
-                element={`${element}_OVERLAY`}
+                data-paste-element={`${element}_OVERLAY`}
                 variant={size}
               >
                 <Box
@@ -152,7 +152,7 @@ const Modal = React.forwardRef<HTMLDivElement, ModalProps>(
                 >
                   {children}
                 </Box>
-              </Box>
+              </ModalDialogOverlay>
             )
         )}
       </ModalContext.Provider>

diff --git a/packages/paste-core/components/separator/__tests__/__snapshots__/index.spec.tsx.snap b/packages/paste-core/components/separator/__tests__/__snapshots__/index.spec.tsx.snap
@@ -30,7 +30,6 @@ exports[`Separator Render should set horizontal margins 1`] = `
       aria-orientation="horizontal"
       class="emotion-0"
       data-paste-element="SEPARATOR"
-      width="auto"
     />
   </div>
 </DocumentFragment>
@@ -80,7 +79,6 @@ exports[`Separator Render should set responsive horizontal margins 1`] = `
       aria-orientation="horizontal"
       class="emotion-0"
       data-paste-element="SEPARATOR"
-      width="auto"
     />
   </div>
 </DocumentFragment>
@@ -130,7 +128,6 @@ exports[`Separator Render should set responsive vertical margins 1`] = `
       aria-orientation="vertical"
       class="emotion-0"
       data-paste-element="SEPARATOR"
-      height="auto"
     />
   </div>
 </DocumentFragment>
@@ -166,7 +163,6 @@ exports[`Separator Render should set vertical margins 1`] = `
       aria-orientation="vertical"
       class="emotion-0"
       data-paste-element="SEPARATOR"
-      height="auto"
     />
   </div>
 </DocumentFragment>

diff --git a/packages/paste-core/layout/grid/__tests__/__snapshots__/grid.test.tsx.snap b/packages/paste-core/layout/grid/__tests__/__snapshots__/grid.test.tsx.snap
@@ -14,7 +14,6 @@ exports[`Grid render should render a Column 1`] = `
 <div
     class="emotion-0"
     data-paste-element="COLUMN"
-    width="8.333333333333332%"
   >
     child
   </div>
@@ -40,7 +39,6 @@ exports[`Grid render should render a Grid 1`] = `
 <div
     class="emotion-0"
     data-paste-element="GRID"
-    display="flex"
   >
     child
   </div>
@@ -66,7 +64,6 @@ exports[`Grid render should render a Grid as any HTML element 1`] = `
 <section
     class="emotion-0"
     data-paste-element="GRID"
-    display="flex"
   >
     child
   </section>
@@ -175,17 +172,14 @@ exports[`Grid render should render responsive css 1`] = `
 <div
     class="emotion-2"
     data-paste-element="GRID"
-    display="flex"
   >
     <div
       class="emotion-0"
       data-paste-element="COLUMN"
-      width="16.666666666666664%,33.33333333333333%,16.666666666666664%"
     />
     <div
       class="emotion-1"
       data-paste-element="COLUMN"
-      width="50%"
     />
   </div>
 </DocumentFragment>

diff --git a/packages/paste-core/primitives/box/src/CustomStyleProps.ts b/packages/paste-core/primitives/box/src/CustomStyleProps.ts
@@ -0,0 +1,73 @@
+export const customStyleProps = {
+  color: {
+    property: 'color',
+    scale: 'textColors',
+  },
+  backgroundColor: {
+    property: 'backgroundColor',
+    scale: 'backgroundColors',
+  },
+  borderColor: {
+    property: 'borderColor',
+    scale: 'borderColors',
+  },
+  borderBottomColor: {
+    property: 'borderBottomColor',
+    scale: 'borderColors',
+  },
+  borderLeftColor: {
+    property: 'borderLeftColor',
+    scale: 'borderColors',
+  },
+  borderRightColor: {
+    property: 'borderRightColor',
+    scale: 'borderColors',
+  },
+  borderTopColor: {
+    property: 'borderTopColor',
+    scale: 'borderColors',
+  },
+  content: true,
+  cursor: true,
+  appearance: true,
+  transition: true,
+  transform: true,
+  animation: true,
+  transformOrigin: true,
+  visibility: true,
+  userSelect: true,
+  pointerEvents: true,
+  boxSizing: true,
+  resize: true,
+  listStyleType: true,
+  listStylePosition: true,
+  listStyleImage: true,
+  objectFit: true,
+  objectPosition: true,
+  outline: true,
+  float: true,
+  willChange: true,
+  clip: true,
+  backgroundAttachment: true,
+  textAlign: true,
+  textTransform: true,
+  textDecoration: true,
+  textOverflow: true,
+  whiteSpace: true,
+  wordBreak: true,
+  wordWrap: true,
+  overflowWrap: true,
+  opacity: true,
+  borderCollapse: true,
+  borderSpacing: true,
+  tableLayout: true,
+  fontVariantNumeric: true,
+  columnGap: {
+    property: 'columnGap',
+    scale: 'space',
+  },
+  rowGap: {
+    property: 'rowGap',
+    scale: 'space',
+  },
+};