fix(MenuItem): Merge Menu and MenuItem variables

CHANGELOG.md

@@ -40,6 +40,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Fix `active` styles for `iconOnly` `MenuItem` in Teams theme @mnajdova ([#1464](https://github.com/stardust-ui/react/pull/1464))
 - Fix keypress/click handling for `Popup` content @kuzhelov ([#1482](https://github.com/stardust-ui/react/pull/1482))
 - Fix `PopupContent` background color in Teams theme @mnajdova ([#1484](https://github.com/stardust-ui/react/pull/1484))
+- Fix merging of item variables in `Menu` and `Toolbar` @miroslavstastny ([#1447](https://github.com/stardust-ui/react/pull/1447)) 
 
 ### Documentation
 - Remove unfinished themes from the docs themes dropdown on components examples pages @alinais ([#1473](https://github.com/stardust-ui/react/pull/1473)

packages/react/src/components/Menu/Menu.tsx

@@ -13,6 +13,8 @@ import {
   getKindProp,
   rtlTextContainer,
 } from '../../lib'
+import { mergeComponentVariables } from '../../lib/mergeThemes'
+
 import MenuItem from './MenuItem'
 import { menuBehavior } from '../../lib/accessibility'
 import { Accessibility } from '../../lib/accessibility/types'
@@ -125,7 +127,7 @@ class Menu extends AutoControlledComponent<WithAsProp<MenuProps>, MenuState> {
   static Item = MenuItem
   static Divider = MenuDivider
 
-  handleItemOverrides = predefinedProps => ({
+  handleItemOverrides = variables => predefinedProps => ({
     onClick: (e, itemProps) => {
       const { index } = itemProps
 
@@ -142,6 +144,11 @@ class Menu extends AutoControlledComponent<WithAsProp<MenuProps>, MenuState> {
       }
       _.invoke(predefinedProps, 'onActiveChanged', e, props)
     },
+    variables: mergeComponentVariables(variables, predefinedProps.variables),
+  })
+
+  handleDividerOverrides = variables => predefinedProps => ({
+    variables: mergeComponentVariables(variables, predefinedProps.variables),
   })
 
   renderItems = (
@@ -165,6 +172,9 @@ class Menu extends AutoControlledComponent<WithAsProp<MenuProps>, MenuState> {
     const itemsCount = _.filter(items, item => getKindProp(item, 'item') !== 'divider').length
     let itemPosition = 0
 
+    const overrideItemProps = this.handleItemOverrides(variables)
+    const overrideDividerProps = this.handleDividerOverrides(variables)
+
     return _.map(items, (item, index) => {
       const active =
         (typeof activeIndex === 'string' ? parseInt(activeIndex, 10) : activeIndex) === index
@@ -177,13 +187,13 @@ class Menu extends AutoControlledComponent<WithAsProp<MenuProps>, MenuState> {
             primary,
             secondary,
             vertical,
-            variables,
             styles: styles.divider,
             inSubmenu: submenu,
             accessibility: accessibility.childBehaviors
               ? accessibility.childBehaviors.divider
               : undefined,
           },
+          overrideProps: overrideDividerProps,
         })
       }
 
@@ -198,7 +208,6 @@ class Menu extends AutoControlledComponent<WithAsProp<MenuProps>, MenuState> {
           primary,
           secondary,
           underlined,
-          variables,
           vertical,
           index,
           itemPosition,
@@ -210,7 +219,7 @@ class Menu extends AutoControlledComponent<WithAsProp<MenuProps>, MenuState> {
             ? accessibility.childBehaviors.item
             : undefined,
         },
-        overrideProps: this.handleItemOverrides,
+        overrideProps: overrideItemProps,
       })
     })
   }

packages/react/src/components/Toolbar/Toolbar.tsx

@@ -12,6 +12,8 @@ import {
   commonPropTypes,
   ColorComponentProps,
 } from '../../lib'
+import { mergeComponentVariables } from '../../lib/mergeThemes'
+
 import { Accessibility } from '../../lib/accessibility/types'
 import { defaultBehavior } from '../../lib/accessibility'
 import { ShorthandCollection, WithAsProp, withSafeTypeForAs } from '../../types'
@@ -58,10 +60,7 @@ class Toolbar extends UIComponent<WithAsProp<ToolbarProps>, any> {
   static RadioGroup = ToolbarRadioGroup
 
   handleItemOverrides = variables => predefinedProps => ({
-    variables: {
-      ...variables,
-      ...predefinedProps.variables,
-    },
+    variables: mergeComponentVariables(variables, predefinedProps.variables),
   })
 
   renderItems(items, variables) {

packages/react/src/components/Toolbar/ToolbarRadioGroup.tsx

@@ -11,6 +11,8 @@ import {
   childrenExist,
   commonPropTypes,
 } from '../../lib'
+import { mergeComponentVariables } from '../../lib/mergeThemes'
+
 import { ShorthandCollection, WithAsProp, withSafeTypeForAs } from '../../types'
 import { Accessibility } from '../../lib/accessibility/types'
 import { defaultBehavior } from '../../lib/accessibility'
@@ -50,10 +52,7 @@ class ToolbarRadioGroup extends UIComponent<WithAsProp<ToolbarRadioGroupProps>>
   }
 
   handleItemOverrides = variables => predefinedProps => ({
-    variables: {
-      ...variables,
-      ...predefinedProps.variables,
-    },
+    variables: mergeComponentVariables(variables, predefinedProps.variables),
   })
 
   renderItems(items, variables) {

packages/react/test/specs/components/Menu/Menu-test.tsx

@@ -12,6 +12,7 @@ import {
   tabBehavior,
 } from 'src/lib/accessibility'
 import { AccessibilityDefinition } from 'src/lib/accessibility/types'
+import { ReactWrapper } from 'enzyme'
 
 const menuImplementsCollectionShorthandProp = implementsCollectionShorthandProp(Menu)
 
@@ -118,6 +119,60 @@ describe('Menu', () => {
       })
     })
 
+    describe('variables', () => {
+      function checkMergedVariables(menu: ReactWrapper): void {
+        expect(
+          (menu
+            .find('MenuItem')
+            .first()
+            .prop('variables') as Function)(),
+        ).toEqual(expect.objectContaining({ a: 'menu', b: 'overwritten', c: 'item' }))
+
+        expect(
+          (menu
+            .find('MenuDivider')
+            .first()
+            .prop('variables') as Function)(),
+        ).toEqual(expect.objectContaining({ a: 'menu', b: 'overwrittenInDivider', c: 'divider' }))
+      }
+
+      it('are passed from Menu to MenuItem and MenuDivider and correctly merged', () => {
+        const menu = mountWithProvider(
+          <Menu
+            variables={{ a: 'menu', b: 'menu' }}
+            items={[
+              { key: 1, content: 'menu item', variables: { b: 'overwritten', c: 'item' } },
+              {
+                key: 'd1',
+                kind: 'divider',
+                variables: { b: 'overwrittenInDivider', c: 'divider' },
+              },
+            ]}
+          />,
+        )
+
+        checkMergedVariables(menu)
+      })
+
+      it('as functions are passed from Menu to MenuItem and MenuDivider and correctly merged', () => {
+        const menu = mountWithProvider(
+          <Menu
+            variables={() => ({ a: 'menu', b: 'menu' })}
+            items={[
+              { key: 1, content: 'menu item', variables: () => ({ b: 'overwritten', c: 'item' }) },
+              {
+                key: 'd1',
+                kind: 'divider',
+                variables: () => ({ b: 'overwrittenInDivider', c: 'divider' }),
+              },
+            ]}
+          />,
+        )
+
+        checkMergedVariables(menu)
+      })
+    })
+
     describe('accessibility', () => {
       handlesAccessibility(Menu, {
         defaultRootRole: 'menu',

packages/react/test/specs/components/Toolbar/Toolbar-test.tsx

@@ -0,0 +1,81 @@
+import * as React from 'react'
+import Toolbar from 'src/components/Toolbar/Toolbar'
+
+import { isConformant } from 'test/specs/commonTests'
+import { mountWithProvider } from 'test/utils'
+import { ReactWrapper } from 'enzyme'
+
+describe('Toolbar', () => {
+  isConformant(Toolbar)
+
+  describe('variables', () => {
+    function checkMergedVariables(toolbar: ReactWrapper): void {
+      expect(
+        (toolbar
+          .find('ToolbarItem')
+          .first()
+          .prop('variables') as Function)(),
+      ).toEqual(expect.objectContaining({ a: 'toolbar', b: 'overwritten', c: 'item' }))
+
+      expect(
+        (toolbar
+          .find('ToolbarDivider')
+          .first()
+          .prop('variables') as Function)(),
+      ).toEqual(expect.objectContaining({ a: 'toolbar', b: 'overwrittenInDivider', c: 'divider' }))
+
+      expect(
+        (toolbar
+          .find('ToolbarRadioGroup')
+          .first()
+          .prop('variables') as Function)(),
+      ).toEqual(expect.objectContaining({ a: 'toolbar', b: 'overwrittenInGroup', c: 'group' }))
+    }
+
+    it('are passed from Toolbar to all kinds of children and correctly merged', () => {
+      const toolbar = mountWithProvider(
+        <Toolbar
+          variables={{ a: 'toolbar', b: 'toolbar' }}
+          items={[
+            { key: 1, content: 'toolbar item', variables: { b: 'overwritten', c: 'item' } },
+            {
+              key: 'd1',
+              kind: 'divider',
+              variables: { b: 'overwrittenInDivider', c: 'divider' },
+            },
+            {
+              key: 'group',
+              kind: 'group',
+              variables: { b: 'overwrittenInGroup', c: 'group' },
+            },
+          ]}
+        />,
+      )
+
+      checkMergedVariables(toolbar)
+    })
+
+    it('as functions are passed from Toolbar to all kinds of children and correctly merged', () => {
+      const toolbar = mountWithProvider(
+        <Toolbar
+          variables={() => ({ a: 'toolbar', b: 'toolbar' })}
+          items={[
+            { key: 1, content: 'toolbar item', variables: () => ({ b: 'overwritten', c: 'item' }) },
+            {
+              key: 'd1',
+              kind: 'divider',
+              variables: () => ({ b: 'overwrittenInDivider', c: 'divider' }),
+            },
+            {
+              key: 'group',
+              kind: 'group',
+              variables: () => ({ b: 'overwrittenInGroup', c: 'group' }),
+            },
+          ]}
+        />,
+      )
+
+      checkMergedVariables(toolbar)
+    })
+  })
+})

packages/react/test/specs/components/Toolbar/ToolbarRadioGroup-test.tsx

@@ -0,0 +1,70 @@
+import { isConformant } from 'test/specs/commonTests'
+
+import ToolbarRadioGroup from 'src/components/Toolbar/ToolbarRadioGroup'
+import { ReactWrapper } from 'enzyme'
+import { mountWithProvider } from 'test/utils'
+import * as React from 'react'
+
+describe('ToolbarRadioGroup', () => {
+  isConformant(ToolbarRadioGroup)
+
+  describe('variables', () => {
+    function checkMergedVariables(toolbarRadioGroup: ReactWrapper): void {
+      expect(
+        (toolbarRadioGroup
+          .find('ToolbarItem')
+          .first()
+          .prop('variables') as Function)(),
+      ).toEqual(expect.objectContaining({ a: 'toolbarRadioGroup', b: 'overwritten', c: 'item' }))
+
+      expect(
+        (toolbarRadioGroup
+          .find('ToolbarDivider')
+          .first()
+          .prop('variables') as Function)(),
+      ).toEqual(
+        expect.objectContaining({
+          a: 'toolbarRadioGroup',
+          b: 'overwrittenInDivider',
+          c: 'divider',
+        }),
+      )
+    }
+
+    it('are passed from Toolbar to all kinds of children and correctly merged', () => {
+      const toolbarRadioGroup = mountWithProvider(
+        <ToolbarRadioGroup
+          variables={{ a: 'toolbarRadioGroup', b: 'toolbarRadioGroup' }}
+          items={[
+            { key: 1, content: 'toolbar item', variables: { b: 'overwritten', c: 'item' } },
+            {
+              key: 'd1',
+              kind: 'divider',
+              variables: { b: 'overwrittenInDivider', c: 'divider' },
+            },
+          ]}
+        />,
+      )
+
+      checkMergedVariables(toolbarRadioGroup)
+    })
+
+    it('as functions are passed from Toolbar to all kinds of children and correctly merged', () => {
+      const toolbarRadioGroup = mountWithProvider(
+        <ToolbarRadioGroup
+          variables={() => ({ a: 'toolbarRadioGroup', b: 'toolbarRadioGroup' })}
+          items={[
+            { key: 1, content: 'toolbar item', variables: () => ({ b: 'overwritten', c: 'item' }) },
+            {
+              key: 'd1',
+              kind: 'divider',
+              variables: () => ({ b: 'overwrittenInDivider', c: 'divider' }),
+            },
+          ]}
+        />,
+      )
+
+      checkMergedVariables(toolbarRadioGroup)
+    })
+  })
+})