Remove redundant styles in Section and BreakoutBanner components (#677)

* remove redundant styles in section and breakout banner components * Create plenty-tomatoes-greet.md
primer · Aug 2, 2024 · c76c8c8 · c76c8c8
1 parent 7da3471
commit c76c8c8
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 27 deletions.
diff --git a/.changeset/plenty-tomatoes-greet.md b/.changeset/plenty-tomatoes-greet.md
@@ -0,0 +1,5 @@
+---
+"@primer/react-brand": patch
+---
+
+Removed redundant styles in default `Section` and `BreakoutBanner` components
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/react/src/BreakoutBanner/BreakoutBanner.tsx b/packages/react/src/BreakoutBanner/BreakoutBanner.tsx
@@ -93,15 +93,22 @@ const Root = forwardRef(
       [processBackgroundValue],
     )
 
-    const backgroundStyles = useMemo(
-      () => ({
-        ...createStyles('background-color', backgroundColor),
-        ...createStyles('background-image-src', backgroundImageSrc),
-        ...createStyles('background-image-position', backgroundImagePosition),
-        ...createStyles('background-image-size', backgroundImageSize),
-      }),
-      [backgroundColor, backgroundImageSrc, backgroundImagePosition, backgroundImageSize, createStyles],
-    )
+    const backgroundStyles = useMemo(() => {
+      const allStyles = {}
+
+      const addStyle = (property, value) => {
+        if (value) {
+          Object.assign(allStyles, createStyles(property, value))
+        }
+      }
+
+      addStyle('background-color', backgroundColor)
+      addStyle('background-image-src', backgroundImageSrc)
+      addStyle('background-image-position', backgroundImagePosition)
+      addStyle('background-image-size', backgroundImageSize)
+
+      return allStyles
+    }, [backgroundColor, backgroundImageSrc, backgroundImagePosition, backgroundImageSize, createStyles])
 
     const hasHeading = useMemo(
       () => React.Children.toArray(children).some(child => React.isValidElement(child) && child.type === _Heading),

diff --git a/packages/react/src/Section/Section.features.stories.tsx b/packages/react/src/Section/Section.features.stories.tsx
@@ -115,7 +115,7 @@ export const CustomBackgroundColor: StoryFn<typeof Section> = () => {
   )
 }
 
-export const ResponsivemBackgroundColor: StoryFn<typeof Section> = () => {
+export const ResponsiveBackgroundColor: StoryFn<typeof Section> = () => {
   return (
     <>
       <Section backgroundColor={{narrow: 'subtle', regular: '#F8F8Ff', wide: '#FFF8F8'}}>

diff --git a/packages/react/src/Section/Section.tsx b/packages/react/src/Section/Section.tsx
@@ -132,6 +132,15 @@ export const Section = forwardRef<HTMLDivElement, PropsWithChildren<SectionProps
       [processBackgroundValue],
     )
 
+    const addStyle = useCallback(
+      (obj, property, value) => {
+        if (value) {
+          Object.assign(obj, createStyles(property, value))
+        }
+      },
+      [createStyles],
+    )
+
     const paddingBlockStartClass = useMemo(
       () => createPaddingClasses(paddingBlockStart, 'paddingBlockStart'),
       [paddingBlockStart, createPaddingClasses],
@@ -141,15 +150,16 @@ export const Section = forwardRef<HTMLDivElement, PropsWithChildren<SectionProps
       [paddingBlockEnd, createPaddingClasses],
     )
 
-    const backgroundStyles = useMemo(
-      () => ({
-        ...createStyles('background-color', backgroundColor),
-        ...createStyles('background-image-src', backgroundImageSrc),
-        ...createStyles('background-image-position', backgroundImagePosition),
-        ...createStyles('background-image-size', backgroundImageSize),
-      }),
-      [backgroundColor, backgroundImageSrc, backgroundImagePosition, backgroundImageSize, createStyles],
-    )
+    const backgroundStyles = useMemo(() => {
+      const allStyles = {}
+
+      addStyle(allStyles, 'background-color', backgroundColor)
+      addStyle(allStyles, 'background-image-src', backgroundImageSrc)
+      addStyle(allStyles, 'background-image-position', backgroundImagePosition)
+      addStyle(allStyles, 'background-image-size', backgroundImageSize)
+
+      return allStyles
+    }, [addStyle, backgroundColor, backgroundImageSrc, backgroundImagePosition, backgroundImageSize])
 
     return (
       <Component

diff --git a/packages/react/src/Section/Section.visual.spec.ts b/packages/react/src/Section/Section.visual.spec.ts
@@ -84,9 +84,9 @@ test.describe('Visual Comparison: Section', () => {
     expect(await page.screenshot()).toMatchSnapshot()
   })
 
-  test('Section / Responsivem Background Color', async ({page}) => {
+  test('Section / Responsive Background Color', async ({page}) => {
     await page.goto(
-      'http://localhost:6006/iframe.html?args=&id=components-section-features--responsivem-background-color&viewMode=story',
+      'http://localhost:6006/iframe.html?args=&id=components-section-features--responsive-background-color&viewMode=story',
     )
 
     await page.waitForTimeout(500)

diff --git a/...-Responsivem-Background-Color-1-linux.png → ...n-Responsive-Background-Color-1-linux.png b/...-Responsivem-Background-Color-1-linux.png → ...n-Responsive-Background-Color-1-linux.png