Repository: uber-web/baseui (js)

Issue:
Allow adding a prefix to the href attribute of the side-nav
Currently the Link's href is set to item.itemId in the side-nav component. This makes it cumbersome if all Links need to be prefixed.

I can take this up if needed.

Pull request:
feat(side-nav): [wip] add mapItem to side-nav
Fixes #1139

#### Description

This allows the user to perform a basic transform on the items array.
Example usecase: add a prefix to each itemId

#### Scope
- [x] Minor: New Feature

Patch:
diff --git a/src/side-navigation/nav.js b/src/side-navigation/nav.js
index e3c0af8c4..732e2c6e9 100644
--- a/src/side-navigation/nav.js
+++ b/src/side-navigation/nav.js
@@ -23,6 +23,7 @@ export default class SideNav extends React.Component<NavPropsT> {
     items: [],
     overrides: {},
     renderItem: null,
+    mapItem: null,
   };
 
   activePredicate = (item: Item) => item.itemId === this.props.activeItemId;
@@ -35,6 +36,7 @@ export default class SideNav extends React.Component<NavPropsT> {
       onChange,
       overrides,
       renderItem,
+      mapItem,
     } = this.props;
     const navLevel = 1;
 
@@ -48,7 +50,18 @@ export default class SideNav extends React.Component<NavPropsT> {
       StyledSubNavContainer,
     );
 
-    const renderNavItem = (item: Item, level: number, index) => {
+    const renderNavItem = (item: Item, level: number, index, mapItem) => {
+      if (typeof mapItem === 'function') {
+        const recMapItem = item => {
+          let subnav = [];
+          if (item.subnav) {
+            subnav = item.subnav.map(recMapItem);
+          }
+          return mapItem({...item, subnav: subnav});
+        };
+        item = recMapItem(item);
+      }
+
       const sharedProps = {
         $active: activePredicate
           ? activePredicate(item, activeItemId)
@@ -56,6 +69,7 @@ export default class SideNav extends React.Component<NavPropsT> {
         $level: level,
         $selectable: !!item.itemId,
       };
+
       return (
         <NavItemContainer
           key={`${index}-level${level}-${
@@ -86,7 +100,7 @@ export default class SideNav extends React.Component<NavPropsT> {
     return (
       <Root role="list" {...rootProps}>
         {items.map((item, index) => {
-          return renderNavItem(item, navLevel, index);
+          return renderNavItem(item, navLevel, index, mapItem);
         })}
       </Root>
     );
diff --git a/src/side-navigation/types.js b/src/side-navigation/types.js
index 17741927d..a1b6051b6 100644
--- a/src/side-navigation/types.js
+++ b/src/side-navigation/types.js
@@ -43,9 +43,9 @@ type renderItemT = (
 export type NavPropsT = {
   /** Defines the current active itemId. Used for the default calculation of the $active prop */
   activeItemId: string,
-  /** 
-    Is called on the nav item render to test if the item is currently selected. 
-    If returns true the item will be rendered as an active one 
+  /**
+    Is called on the nav item render to test if the item is currently selected.
+    If returns true the item will be rendered as an active one
     */
   activePredicate: ?(item: *, activeItemId: string) => boolean,
   /** List of navigation items */
@@ -62,21 +62,23 @@ export type NavPropsT = {
   },
   /** Optional render function that is called instead default item rendering */
   renderItem: ?renderItemT,
+  /** Optional transform function that is called for each Item */
+  mapItem: ?(item: Item) => Item,
 };
 
-export type Item = {
+export type Item = $ReadOnly<{
   /** Navigation item's title to render */
   title: React.Node,
-  /** 
-    Identifier for the navigation item. 
-    Can be a path value or an action name. 
+  /**
+    Identifier for the navigation item.
+    Can be a path value or an action name.
     It's also used in the default `activePredicate` to
     identify a currently active item
     */
   itemId?: string,
   /** A list of sub-navigation items */
   subnav?: Item[],
-};
+}>;
 
 export type NavItemPropsT = SharedPropsT & {
   item: Item,


Test patch:
diff --git a/src/side-navigation/__tests__/__snapshots__/nav.test.js.snap b/src/side-navigation/__tests__/__snapshots__/nav.test.js.snap
new file mode 100644
index 000000000..304ff8379
--- /dev/null
+++ b/src/side-navigation/__tests__/__snapshots__/nav.test.js.snap
@@ -0,0 +1,26 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Side navigation renders titles correctly modified by mapItem 1`] = `
+Array [
+  <span>
+    New 
+    Colors
+  </span>,
+  <span>
+    New 
+    Shades
+  </span>,
+  <span>
+    New 
+    Light
+  </span>,
+  <span>
+    New 
+    Sizing
+  </span>,
+  <span>
+    New 
+    Typography
+  </span>,
+]
+`;
diff --git a/src/side-navigation/__tests__/__snapshots__/stateful-nav.test.js.snap b/src/side-navigation/__tests__/__snapshots__/stateful-nav.test.js.snap
index 4678c5d35..7bf6a8ae0 100644
--- a/src/side-navigation/__tests__/__snapshots__/stateful-nav.test.js.snap
+++ b/src/side-navigation/__tests__/__snapshots__/stateful-nav.test.js.snap
@@ -31,6 +31,7 @@ exports[`Navigation basic render: renders <Navigation/> as a child 1`] = `
       },
     ]
   }
+  mapItem={null}
   onChange={[Function]}
   overrides={Object {}}
   renderItem={null}
diff --git a/src/side-navigation/__tests__/nav.test.js b/src/side-navigation/__tests__/nav.test.js
index b7075d27d..58a98b6fc 100644
--- a/src/side-navigation/__tests__/nav.test.js
+++ b/src/side-navigation/__tests__/nav.test.js
@@ -7,7 +7,7 @@ LICENSE file in the root directory of this source tree.
 // @flow
 import React from 'react';
 import {mount} from 'enzyme';
-import {Navigation, StyledNavItemContainer} from '../index.js';
+import {Navigation, StyledNavItemContainer, NavItem} from '../index.js';
 
 const nav = [
   {
@@ -40,4 +40,22 @@ describe('Side navigation', () => {
     const wrapper = mount(<Navigation items={nav} />);
     expect(wrapper.find(StyledNavItemContainer)).toHaveLength(5);
   });
+
+  it('renders titles correctly modified by mapItem', () => {
+    const mapItem = item => ({
+      ...item,
+      title: <span>New {item.title}</span>,
+    });
+    const wrapper = mount(<Navigation items={nav} mapItem={mapItem} />);
+    expect(
+      wrapper.find(NavItem).map(item => item.instance().props.item.title),
+    ).toMatchSnapshot();
+  });
+
+  it('calls mapItem exactly once for each item', () => {
+    const mapItem = jest.fn();
+    mapItem.mockImplementation(item => item);
+    mount(<Navigation items={nav} mapItem={mapItem} />);
+    expect(mapItem).toHaveBeenCalledTimes(5);
+  });
 });