From 90794fcb59a5ec62c687909f6f72b907eb3731f3 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Mon, 15 Jun 2020 18:55:44 +0200
Subject: [PATCH 1/3] packages/core: fix base path being ignored + sidebar
 highlighting

---
 packages/app/src/components/Root/Root.tsx     | 12 +++----
 packages/core-api/src/app/App.tsx             |  9 ++++-
 packages/core/src/layout/Sidebar/Items.tsx    | 36 ++++++++-----------
 .../Sidebar/Settings/ProviderSettingsItem.tsx |  7 +---
 .../layout/Sidebar/Settings/UserProfile.tsx   |  1 -
 5 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/packages/app/src/components/Root/Root.tsx b/packages/app/src/components/Root/Root.tsx
index 8b1ce76cbd..453c98bc72 100644
--- a/packages/app/src/components/Root/Root.tsx
+++ b/packages/app/src/components/Root/Root.tsx
@@ -86,14 +86,14 @@ const Root: FC<{}> = ({ children }) => (
       <SidebarSearchField onSearch={handleSearch} />
       <SidebarDivider />
       {/* Global nav, not org-specific */}
-      <SidebarItem icon={HomeIcon} to="/" text="Home" />
-      <SidebarItem icon={ExploreIcon} to="/explore" text="Explore" />
-      <SidebarItem icon={CreateComponentIcon} to="/create" text="Create..." />
+      <SidebarItem icon={HomeIcon} to="./" text="Home" />
+      <SidebarItem icon={ExploreIcon} to="explore" text="Explore" />
+      <SidebarItem icon={CreateComponentIcon} to="create" text="Create..." />
       {/* End global nav */}
       <SidebarDivider />
-      <SidebarItem icon={MapIcon} to="/tech-radar" text="Tech Radar" />
-      <SidebarItem icon={RuleIcon} to="/lighthouse" text="Lighthouse" />
-      <SidebarItem icon={BuildIcon} to="/circleci" text="CircleCI" />
+      <SidebarItem icon={MapIcon} to="tech-radar" text="Tech Radar" />
+      <SidebarItem icon={RuleIcon} to="lighthouse" text="Lighthouse" />
+      <SidebarItem icon={BuildIcon} to="circleci" text="CircleCI" />
       <SidebarSpace />
       <SidebarDivider />
       <SidebarThemeToggle />
diff --git a/packages/core-api/src/app/App.tsx b/packages/core-api/src/app/App.tsx
index d7920e6bbd..c5c53f50ba 100644
--- a/packages/core-api/src/app/App.tsx
+++ b/packages/core-api/src/app/App.tsx
@@ -205,7 +205,14 @@ export class PrivateAppImpl implements BackstageApp {
         <ApiProvider apis={apis}>
           <AppContextProvider app={this}>
             <AppThemeProvider>
-              <Router>{children}</Router>
+              <Router>
+                <Routes>
+                  <Route
+                    path={`${pathname}/*`}
+                    element={<div>{children}</div>}
+                  />
+                </Routes>
+              </Router>
             </AppThemeProvider>
           </AppContextProvider>
         </ApiProvider>
diff --git a/packages/core/src/layout/Sidebar/Items.tsx b/packages/core/src/layout/Sidebar/Items.tsx
index 5c2909f82e..68f67c1445 100644
--- a/packages/core/src/layout/Sidebar/Items.tsx
+++ b/packages/core/src/layout/Sidebar/Items.tsx
@@ -111,8 +111,8 @@ const useStyles = makeStyles<Theme>(theme => {
 type SidebarItemProps = {
   icon: IconComponent;
   text?: string;
+  // If 'to' is set the item will act as a nav link with highlight, otherwise it's just a button
   to?: string;
-  disableSelected?: boolean;
   hasNotifications?: boolean;
   onClick?: () => void;
 };
@@ -120,9 +120,7 @@ type SidebarItemProps = {
 export const SidebarItem: FC<SidebarItemProps> = ({
   icon: Icon,
   text,
-  to = '#',
-  // TODO: isActive is not in v6
-  // disableSelected = false,
+  to,
   hasNotifications = false,
   onClick,
   children,
@@ -144,27 +142,23 @@ export const SidebarItem: FC<SidebarItemProps> = ({
     </Badge>
   );
 
+  const Component = to === undefined ? 'div' : NavLink;
+  const childProps = {
+    activeClassName: classes.selected,
+    end: true,
+    to: to!,
+    onClick: onClick,
+  };
+
   if (!isOpen) {
     return (
-      <NavLink
-        className={clsx(classes.root, classes.closed)}
-        activeClassName={classes.selected}
-        end
-        to={to}
-        onClick={onClick}
-      >
+      <Component className={clsx(classes.root, classes.closed)} {...childProps}>
         {itemIcon}
-      </NavLink>
+      </Component>
     );
   }
   return (
-    <NavLink
-      className={clsx(classes.root, classes.open)}
-      activeClassName={classes.selected}
-      end
-      to={to}
-      onClick={onClick}
-    >
+    <Component className={clsx(classes.root, classes.open)} {...childProps}>
       <div data-testid="login-button" className={classes.iconContainer}>
         {itemIcon}
       </div>
@@ -174,7 +168,7 @@ export const SidebarItem: FC<SidebarItemProps> = ({
         </Typography>
       )}
       <div className={classes.secondaryAction}>{children}</div>
-    </NavLink>
+    </Component>
   );
 };
 
@@ -198,7 +192,7 @@ export const SidebarSearchField: FC<SidebarSearchFieldProps> = props => {
 
   return (
     <div className={classes.searchRoot}>
-      <SidebarItem icon={SearchIcon} disableSelected>
+      <SidebarItem icon={SearchIcon}>
         <TextField
           placeholder="Search"
           onChange={handleInput}
diff --git a/packages/core/src/layout/Sidebar/Settings/ProviderSettingsItem.tsx b/packages/core/src/layout/Sidebar/Settings/ProviderSettingsItem.tsx
index 8fbc945cc2..3d3d7439e1 100644
--- a/packages/core/src/layout/Sidebar/Settings/ProviderSettingsItem.tsx
+++ b/packages/core/src/layout/Sidebar/Settings/ProviderSettingsItem.tsx
@@ -29,12 +29,7 @@ export const ProviderSettingsItem: FC<{
   signInHandler: Function;
 }> = ({ title, icon, signedIn, api, signInHandler }) => {
   return (
-    <SidebarItem
-      key={title}
-      text={title}
-      icon={icon ?? StarBorder}
-      disableSelected
-    >
+    <SidebarItem key={title} text={title} icon={icon ?? StarBorder}>
       <IconButton onClick={() => (signedIn ? api.logout() : signInHandler())}>
         <Tooltip
           placement="top"
diff --git a/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx b/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx
index ebf1d0877d..9cc122e2fa 100644
--- a/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx
+++ b/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx
@@ -103,7 +103,6 @@ export const UserProfile: FC<{ open: boolean; setOpen: Function }> = ({
         text={displayName}
         onClick={handleClick}
         icon={avatar || AccountCircleIcon}
-        disableSelected
       >
         {open ? <ExpandLess /> : <ExpandMore />}
       </SidebarItem>

From 972745a0914c3f4b3eb4b7498b6139baaa0b239e Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Mon, 15 Jun 2020 19:23:34 +0200
Subject: [PATCH 2/3] packages/core: refactor SidebarItem to avoid warnings

---
 packages/core/src/layout/Sidebar/Items.tsx | 32 +++++++++++++++-------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/packages/core/src/layout/Sidebar/Items.tsx b/packages/core/src/layout/Sidebar/Items.tsx
index 68f67c1445..a7f40f56ba 100644
--- a/packages/core/src/layout/Sidebar/Items.tsx
+++ b/packages/core/src/layout/Sidebar/Items.tsx
@@ -142,23 +142,25 @@ export const SidebarItem: FC<SidebarItemProps> = ({
     </Badge>
   );
 
-  const Component = to === undefined ? 'div' : NavLink;
   const childProps = {
-    activeClassName: classes.selected,
-    end: true,
-    to: to!,
-    onClick: onClick,
+    onClick,
+    className: clsx(classes.root, isOpen ? classes.open : classes.closed),
   };
 
   if (!isOpen) {
+    if (to === undefined) {
+      return <div {...childProps}>{itemIcon}</div>;
+    }
+
     return (
-      <Component className={clsx(classes.root, classes.closed)} {...childProps}>
+      <NavLink {...childProps} activeClassName={classes.selected} to={to} end>
         {itemIcon}
-      </Component>
+      </NavLink>
     );
   }
-  return (
-    <Component className={clsx(classes.root, classes.open)} {...childProps}>
+
+  const content = (
+    <>
       <div data-testid="login-button" className={classes.iconContainer}>
         {itemIcon}
       </div>
@@ -168,7 +170,17 @@ export const SidebarItem: FC<SidebarItemProps> = ({
         </Typography>
       )}
       <div className={classes.secondaryAction}>{children}</div>
-    </Component>
+    </>
+  );
+
+  if (to === undefined) {
+    return <div {...childProps}>{content}</div>;
+  }
+
+  return (
+    <NavLink {...childProps} activeClassName={classes.selected} to={to} end>
+      {content}
+    </NavLink>
   );
 };
 

From 6692243b20b2b7a59bad0d74372307872eb9631a Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Mon, 15 Jun 2020 19:25:11 +0200
Subject: [PATCH 3/3] packages/core-api: use fragment instead of div to wrap
 app route

---
 packages/core-api/src/app/App.tsx | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/packages/core-api/src/app/App.tsx b/packages/core-api/src/app/App.tsx
index c5c53f50ba..1923adba0d 100644
--- a/packages/core-api/src/app/App.tsx
+++ b/packages/core-api/src/app/App.tsx
@@ -207,10 +207,7 @@ export class PrivateAppImpl implements BackstageApp {
             <AppThemeProvider>
               <Router>
                 <Routes>
-                  <Route
-                    path={`${pathname}/*`}
-                    element={<div>{children}</div>}
-                  />
+                  <Route path={`${pathname}/*`} element={<>{children}</>} />
                 </Routes>
               </Router>
             </AppThemeProvider>