NavBar: Styling tweaks to tidy up appearance (#39871)

* NavBar: Styling tweaks to tidy up appearance * NavBar: Add external link icon to external links * NavBar: Dim the external link icon * bump drone * NavBar: Rename variable to better describe what it's doing
4 years ago · 4350f4931d
parent 4dadb8fc51
commit 4350f4931d
8 changed files with 102 additions and 70 deletions
--- a/packages/grafana-data/src/themes/createComponents.ts
+++ b/packages/grafana-data/src/themes/createComponents.ts
@ -80,7 +80,7 @@ export function createComponents(colors: ThemeColors, shadows: ThemeShadows): Th
      background: colors.mode === 'dark' ? 'rgba(0, 0, 0, 0.45)' : 'rgba(208, 209, 211, 0.24)',
    },
    sidemenu: {
-      width: 60,
+      width: 44,
    },
  };
 }
--- a/public/app/core/components/NavBar/BottomSection.tsx
+++ b/public/app/core/components/NavBar/BottomSection.tsx
@ -101,7 +101,8 @@ const getStyles = (theme: GrafanaTheme2) => ({
    display: none;

    ${theme.breakpoints.up('md')} {
-      display: block;
+      display: flex;
+      flex-direction: inherit;
      margin-bottom: ${theme.spacing(2)};
    }

--- a/public/app/core/components/NavBar/DropdownChild.test.tsx
+++ b/public/app/core/components/NavBar/DropdownChild.test.tsx
@ -31,6 +31,12 @@ describe('DropdownChild', () => {
    expect(icon).toBeInTheDocument();
  });

+  it('displays an external link icon if the target is _blank', () => {
+    render(<DropdownChild text={mockText} icon={mockIcon} url={mockUrl} target="_blank" />);
+    const icon = screen.getByTestId('external-link-icon');
+    expect(icon).toBeInTheDocument();
+  });
+
  it('displays a divider instead when isDivider is true', () => {
    render(<DropdownChild text={mockText} icon={mockIcon} url={mockUrl} isDivider />);

--- a/public/app/core/components/NavBar/DropdownChild.tsx
+++ b/public/app/core/components/NavBar/DropdownChild.tsx
@ -17,10 +17,15 @@ const DropdownChild = ({ isDivider = false, icon, onClick, target, text, url }:
  const styles = getStyles(theme);

  const linkContent = (
-    <>
-      {icon && <Icon data-testid="dropdown-child-icon" name={icon} className={styles.icon} />}
-      {text}
-    </>
+    <div className={styles.linkContent}>
+      <div>
+        {icon && <Icon data-testid="dropdown-child-icon" name={icon} className={styles.icon} />}
+        {text}
+      </div>
+      {target === '_blank' && (
+        <Icon data-testid="external-link-icon" name="external-link-alt" className={styles.externalLinkIcon} />
+      )}
+    </div>
  );

  let element = (
@ -53,7 +58,17 @@ const getStyles = (theme: GrafanaTheme2) => ({
    display: flex;
    width: 100%;
  `,
+  externalLinkIcon: css`
+    color: ${theme.colors.text.secondary};
+    margin-left: ${theme.spacing(1)};
+  `,
  icon: css`
    margin-right: ${theme.spacing(1)};
  `,
+  linkContent: css`
+    display: flex;
+    flex: 1;
+    flex-direction: row;
+    justify-content: space-between;
+  `,
 });
--- a/public/app/core/components/NavBar/NavBar.tsx
+++ b/public/app/core/components/NavBar/NavBar.tsx
@ -49,16 +49,16 @@ NavBar.displayName = 'NavBar';

 const getStyles = (theme: GrafanaTheme2) => ({
  sidemenu: css`
-    border-right: 1px solid ${theme.components.panel.borderColor};
    display: flex;
    flex-direction: column;
    position: fixed;
-    width: ${theme.components.sidemenu.width}px;
    z-index: ${theme.zIndex.sidemenu};

    ${theme.breakpoints.up('md')} {
      background-color: ${theme.colors.background.primary};
+      border-right: 1px solid ${theme.components.panel.borderColor};
      position: relative;
+      width: ${theme.components.sidemenu.width}px;
    }

    .sidemenu-hidden & {
--- a/public/app/core/components/NavBar/NavBarDropdown.tsx
+++ b/public/app/core/components/NavBar/NavBarDropdown.tsx
@ -23,8 +23,9 @@ const NavBarDropdown = ({
  reverseDirection = false,
  subtitleText,
 }: Props) => {
+  const filteredItems = items.filter((item) => !item.hideFromMenu);
  const theme = useTheme2();
-  const styles = getStyles(theme, reverseDirection);
+  const styles = getStyles(theme, reverseDirection, filteredItems);

  let header = (
    <button onClick={onHeaderClick} className={styles.header}>
@ -47,19 +48,17 @@ const NavBarDropdown = ({
  return (
    <ul className={`${styles.menu} dropdown-menu dropdown-menu--sidemenu`} role="menu">
      <li>{header}</li>
-      {items
-        .filter((item) => !item.hideFromMenu)
-        .map((child, index) => (
-          <DropdownChild
-            key={`${child.url}-${index}`}
-            isDivider={child.divider}
-            icon={child.icon as IconName}
-            onClick={child.onClick}
-            target={child.target}
-            text={child.text}
-            url={child.url}
-          />
-        ))}
+      {filteredItems.map((child, index) => (
+        <DropdownChild
+          key={`${child.url}-${index}`}
+          isDivider={child.divider}
+          icon={child.icon as IconName}
+          onClick={child.onClick}
+          target={child.target}
+          text={child.text}
+          url={child.url}
+        />
+      ))}
      {subtitleText && <li className={styles.subtitle}>{subtitleText}</li>}
    </ul>
  );
@ -67,52 +66,62 @@ const NavBarDropdown = ({

 export default NavBarDropdown;

-const getStyles = (theme: GrafanaTheme2, reverseDirection: Props['reverseDirection']) => ({
-  header: css`
-    background-color: ${theme.colors.background.secondary};
-    border: none;
-    color: ${theme.colors.text.primary};
-    font-size: ${theme.typography.h4.fontSize};
-    font-weight: ${theme.typography.h4.fontWeight};
-    padding: ${theme.spacing(1)} ${theme.spacing(1)} ${theme.spacing(1)} ${theme.spacing(2)} !important;
-    white-space: nowrap;
-    width: 100%;
+const getStyles = (
+  theme: GrafanaTheme2,
+  reverseDirection: Props['reverseDirection'],
+  filteredItems: Props['items']
+) => {
+  const adjustHeightForBorder = filteredItems!.length === 0;

-    &:hover {
-      background-color: ${theme.colors.action.hover};
-    }
+  return {
+    header: css`
+      background-color: ${theme.colors.background.secondary};
+      border: none;
+      color: ${theme.colors.text.primary};
+      height: ${theme.components.sidemenu.width - (adjustHeightForBorder ? 2 : 1)}px;
+      font-size: ${theme.typography.h4.fontSize};
+      font-weight: ${theme.typography.h4.fontWeight};
+      padding: ${theme.spacing(1)} ${theme.spacing(1)} ${theme.spacing(1)} ${theme.spacing(2)} !important;
+      white-space: nowrap;
+      width: 100%;
+
+      &:hover {
+        background-color: ${theme.colors.action.hover};
+      }

-    .sidemenu-open--xs & {
-      display: flex;
-      font-size: ${theme.typography.body.fontSize};
-      font-weight: ${theme.typography.body.fontWeight};
-      padding-left: ${theme.spacing(1)} !important;
-    }
-  `,
-  menu: css`
-    flex-direction: ${reverseDirection ? 'column-reverse' : 'column'};
+      .sidemenu-open--xs & {
+        display: flex;
+        font-size: ${theme.typography.body.fontSize};
+        font-weight: ${theme.typography.body.fontWeight};
+        padding-left: ${theme.spacing(1)} !important;
+      }
+    `,
+    menu: css`
+      border: 1px solid ${theme.components.panel.borderColor};
+      flex-direction: ${reverseDirection ? 'column-reverse' : 'column'};

-    .sidemenu-open--xs & {
-      display: flex;
-      flex-direction: column;
-      float: none;
+      .sidemenu-open--xs & {
+        display: flex;
+        flex-direction: column;
+        float: none;
+        margin-bottom: ${theme.spacing(1)};
+        position: unset;
+        width: 100%;
+      }
+    `,
+    subtitle: css`
+      border-bottom: 1px solid ${theme.colors.border.weak};
+      color: ${theme.colors.text.secondary};
+      font-size: ${theme.typography.bodySmall.fontSize};
+      font-weight: ${theme.typography.bodySmall.fontWeight};
      margin-bottom: ${theme.spacing(1)};
-      position: unset;
-      width: 100%;
-    }
-  `,
-  subtitle: css`
-    border-bottom: 1px solid ${theme.colors.border.weak};
-    color: ${theme.colors.text.secondary};
-    font-size: ${theme.typography.bodySmall.fontSize};
-    font-weight: ${theme.typography.bodySmall.fontWeight};
-    margin-bottom: ${theme.spacing(1)};
-    padding: ${theme.spacing(1)} ${theme.spacing(2)} ${theme.spacing(1)};
-    white-space: nowrap;
+      padding: ${theme.spacing(1)} ${theme.spacing(2)} ${theme.spacing(1)};
+      white-space: nowrap;

-    .sidemenu-open--xs & {
-      border-bottom: none;
-      margin-bottom: 0;
-    }
-  `,
-});
+      .sidemenu-open--xs & {
+        border-bottom: none;
+        margin-bottom: 0;
+      }
+    `,
+  };
+};
--- a/public/app/core/components/NavBar/NavBarItem.tsx
+++ b/public/app/core/components/NavBar/NavBarItem.tsx
@ -95,7 +95,6 @@ const getStyles = (theme: GrafanaTheme2, isActive: Props['isActive']) => ({

        .dropdown-menu {
          animation: dropdown-anim 150ms ease-in-out 100ms forwards;
-          border: none;
          display: flex;
          // important to overlap it otherwise it can be hidden
          // again by the mouse getting outside the hover space
@ -117,9 +116,10 @@ const getStyles = (theme: GrafanaTheme2, isActive: Props['isActive']) => ({
    border: none;
    color: inherit;
    display: block;
-    line-height: 42px;
+    line-height: ${theme.components.sidemenu.width}px;
+    padding: 0;
    text-align: center;
-    width: ${theme.components.sidemenu.width - 1}px;
+    width: ${theme.components.sidemenu.width}px;

    &::before {
      display: ${isActive ? 'block' : 'none'};
--- a/public/app/core/components/NavBar/TopSection.tsx
+++ b/public/app/core/components/NavBar/TopSection.tsx
@ -53,7 +53,8 @@ const getStyles = (theme: GrafanaTheme2) => ({
    flex-grow: 1;

    ${theme.breakpoints.up('md')} {
-      display: block;
+      display: flex;
+      flex-direction: inherit;
      margin-top: ${theme.spacing(5)};
    }