Scopes: Highlight current active item/dashboard (#104403)

* Highlight current active item * Add error boundary for scopes selector * Expand containing folder of active item * Add tests
2025-04-30 12:25:12 +02:00
parent 5a589bb51a
commit 3732ec74e7
6 changed files with 391 additions and 18 deletions
@@ -5,7 +5,7 @@ import { PropsWithChildren, useEffect } from 'react';

 import { GrafanaTheme2 } from '@grafana/data';
 import { locationSearchToObject, locationService, useScopes } from '@grafana/runtime';
-import { getDragStyles, LinkButton, useStyles2 } from '@grafana/ui';
+import { ErrorBoundaryAlert, getDragStyles, LinkButton, useStyles2 } from '@grafana/ui';
 import { useGrafana } from 'app/core/context/GrafanaContext';
 import { useMediaQueryMinWidth } from 'app/core/hooks/useMediaQueryMinWidth';
 import { Trans } from 'app/core/internationalization';
@@ -122,7 +122,9 @@ export function AppChrome({ children }: Props) {
                [styles.scopesDashboardsContainerDocked]: menuDockedAndOpen,
              })}
            >
-              <ScopesDashboards />
+              <ErrorBoundaryAlert>
+                <ScopesDashboards />
+              </ErrorBoundaryAlert>
            </div>
          )}
          <main
@@ -0,0 +1,326 @@
+import { Location } from 'history';
+
+import { ScopeDashboardBinding } from '@grafana/data';
+import { config, locationService } from '@grafana/runtime';
+
+import { ScopesApiClient } from '../ScopesApiClient';
+
+import { ScopesDashboardsService } from './ScopesDashboardsService';
+import { ScopeNavigation } from './types';
+
+jest.mock('@grafana/runtime', () => ({
+  ...jest.requireActual('@grafana/runtime'),
+  config: {
+    featureToggles: {
+      useScopesNavigationEndpoint: false,
+    },
+  },
+  locationService: {
+    getLocation: jest.fn(),
+  },
+}));
+
+describe('ScopesDashboardsService', () => {
+  let service: ScopesDashboardsService;
+  let mockApiClient: jest.Mocked<ScopesApiClient>;
+
+  beforeEach(() => {
+    mockApiClient = {
+      fetchDashboards: jest.fn(),
+      fetchScopeNavigations: jest.fn(),
+    } as unknown as jest.Mocked<ScopesApiClient>;
+
+    service = new ScopesDashboardsService(mockApiClient);
+  });
+
+  describe('folder expansion based on location', () => {
+    it('should expand folders when current location matches dashboard ID', async () => {
+      // Mock current location to be a dashboard
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/d/dashboard1' } as Location);
+
+      const mockDashboards: ScopeDashboardBinding[] = [
+        {
+          spec: {
+            scope: 'scope1',
+            dashboard: 'dashboard1',
+          },
+          status: {
+            dashboardTitle: 'Test Dashboard',
+            groups: ['group1'],
+          },
+          metadata: {
+            name: 'dashboard1',
+          },
+        },
+      ];
+
+      mockApiClient.fetchDashboards.mockResolvedValue(mockDashboards);
+      await service.fetchDashboards(['scope1']);
+
+      // Verify that the folder is expanded because the current dashboard ID matches
+      expect(service.state.folders[''].folders['group1'].expanded).toBe(true);
+    });
+
+    it('should expand folders when current location matches URL path and navigation endpoint is enabled', async () => {
+      // Enable the navigation endpoint feature toggle
+      config.featureToggles.useScopesNavigationEndpoint = true;
+
+      // Mock current location to match a URL-based navigation
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/test-url' } as Location);
+
+      const mockNavigations: ScopeNavigation[] = [
+        {
+          spec: {
+            scope: 'scope1',
+            url: '/test-url',
+          },
+          status: {
+            title: 'Test URL',
+            groups: ['group1'],
+          },
+          metadata: {
+            name: 'url1',
+          },
+        },
+      ];
+
+      mockApiClient.fetchScopeNavigations.mockResolvedValue(mockNavigations);
+      await service.fetchDashboards(['scope1']);
+
+      // Verify that the folder is expanded because the current URL path matches
+      expect(service.state.folders[''].folders['group1'].expanded).toBe(true);
+
+      // Reset the feature toggle
+      config.featureToggles.useScopesNavigationEndpoint = false;
+    });
+
+    it('should not expand folders when current location does not match any navigation', async () => {
+      // Mock current location to not match any navigation
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/different-path' } as Location);
+
+      const mockDashboards: ScopeDashboardBinding[] = [
+        {
+          spec: {
+            scope: 'scope1',
+            dashboard: 'dashboard1',
+          },
+          status: {
+            dashboardTitle: 'Test Dashboard',
+            groups: ['group1'],
+          },
+          metadata: {
+            name: 'dashboard1',
+          },
+        },
+      ];
+
+      mockApiClient.fetchDashboards.mockResolvedValue(mockDashboards);
+      await service.fetchDashboards(['scope1']);
+
+      // Verify that the folder is not expanded because the current location doesn't match
+      expect(service.state.folders[''].folders['group1'].expanded).toBe(false);
+    });
+
+    it('should expand folders when current location matches nested dashboard path', async () => {
+      // Mock current location to be a nested dashboard path
+      (locationService.getLocation as jest.Mock).mockReturnValue({
+        pathname: '/d/dashboard1/very-important',
+      } as Location);
+
+      const mockDashboards: ScopeDashboardBinding[] = [
+        {
+          spec: {
+            scope: 'scope1',
+            dashboard: 'dashboard1',
+          },
+          status: {
+            dashboardTitle: 'Test Dashboard',
+            groups: ['group1'],
+          },
+          metadata: {
+            name: 'dashboard1',
+          },
+        },
+      ];
+
+      mockApiClient.fetchDashboards.mockResolvedValue(mockDashboards);
+      await service.fetchDashboards(['scope1']);
+
+      // Verify that the folder is expanded because the current path starts with the dashboard ID
+      expect(service.state.folders[''].folders['group1'].expanded).toBe(true);
+    });
+
+    it('should not expand folders containing different dashboards', async () => {
+      // Mock current location to be a specific dashboard
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/d/dashboard1' } as Location);
+
+      const mockDashboards: ScopeDashboardBinding[] = [
+        {
+          spec: {
+            scope: 'scope1',
+            dashboard: 'dashboard1',
+          },
+          status: {
+            dashboardTitle: 'Test Dashboard',
+            groups: ['group1'],
+          },
+          metadata: {
+            name: 'dashboard1',
+          },
+        },
+        {
+          spec: {
+            scope: 'scope1',
+            dashboard: 'dashboard2',
+          },
+          status: {
+            dashboardTitle: 'Another Dashboard',
+            groups: ['group2'],
+          },
+          metadata: {
+            name: 'dashboard2',
+          },
+        },
+      ];
+
+      mockApiClient.fetchDashboards.mockResolvedValue(mockDashboards);
+      await service.fetchDashboards(['scope1']);
+
+      // Verify that only the folder containing the current dashboard is expanded
+      expect(service.state.folders[''].folders['group1'].expanded).toBe(true);
+      expect(service.state.folders[''].folders['group2'].expanded).toBe(false);
+    });
+
+    describe('with useScopesNavigationEndpoint enabled', () => {
+      beforeEach(() => {
+        config.featureToggles.useScopesNavigationEndpoint = true;
+      });
+
+      afterEach(() => {
+        config.featureToggles.useScopesNavigationEndpoint = false;
+      });
+
+      it('should expand folders when current location matches a navigation URL', async () => {
+        // Mock current location to match a navigation URL
+        (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/custom-page' } as Location);
+
+        const mockNavigations: ScopeNavigation[] = [
+          {
+            spec: {
+              scope: 'scope1',
+              url: '/custom-page',
+            },
+            status: {
+              title: 'Custom Page',
+              groups: ['group1'],
+            },
+            metadata: {
+              name: 'nav1',
+            },
+          },
+        ];
+
+        mockApiClient.fetchScopeNavigations.mockResolvedValue(mockNavigations);
+        await service.fetchDashboards(['scope1']);
+
+        // Verify that the folder is expanded because the current URL matches a navigation
+        expect(service.state.folders[''].folders['group1'].expanded).toBe(true);
+      });
+
+      it('should expand folders when current location matches a nested navigation URL', async () => {
+        // Mock current location to match a nested navigation URL
+        (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/custom-page/details' } as Location);
+
+        const mockNavigations: ScopeNavigation[] = [
+          {
+            spec: {
+              scope: 'scope1',
+              url: '/custom-page',
+            },
+            status: {
+              title: 'Custom Page',
+              groups: ['group1'],
+            },
+            metadata: {
+              name: 'nav1',
+            },
+          },
+        ];
+
+        mockApiClient.fetchScopeNavigations.mockResolvedValue(mockNavigations);
+        await service.fetchDashboards(['scope1']);
+
+        // Verify that the folder is expanded because the current URL starts with a navigation URL
+        expect(service.state.folders[''].folders['group1'].expanded).toBe(true);
+      });
+
+      it('should not expand folders when current location does not match any navigation', async () => {
+        // Mock current location to not match any navigation
+        (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/unrelated-page' } as Location);
+
+        const mockNavigations: ScopeNavigation[] = [
+          {
+            spec: {
+              scope: 'scope1',
+              url: '/custom-page',
+            },
+            status: {
+              title: 'Custom Page',
+              groups: ['group1'],
+            },
+            metadata: {
+              name: 'nav1',
+            },
+          },
+        ];
+
+        mockApiClient.fetchScopeNavigations.mockResolvedValue(mockNavigations);
+        await service.fetchDashboards(['scope1']);
+
+        // Verify that the folder is not expanded because the current URL doesn't match any navigation
+        expect(service.state.folders[''].folders['group1'].expanded).toBe(false);
+      });
+
+      it('should not expand folders containing different navigations', async () => {
+        // Mock current location to match a specific navigation
+        (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/custom-page' } as Location);
+
+        const mockNavigations: ScopeNavigation[] = [
+          {
+            spec: {
+              scope: 'scope1',
+              url: '/custom-page',
+            },
+            status: {
+              title: 'Custom Page',
+              groups: ['group1'],
+            },
+            metadata: {
+              name: 'nav1',
+            },
+          },
+          {
+            spec: {
+              scope: 'scope1',
+              url: '/other-page',
+            },
+            status: {
+              title: 'Other Page',
+              groups: ['group2'],
+            },
+            metadata: {
+              name: 'nav2',
+            },
+          },
+        ];
+
+        mockApiClient.fetchScopeNavigations.mockResolvedValue(mockNavigations);
+        await service.fetchDashboards(['scope1']);
+
+        // Verify that only the folder containing the current navigation is expanded
+        expect(service.state.folders[''].folders['group1'].expanded).toBe(true);
+        expect(service.state.folders[''].folders['group2'].expanded).toBe(false);
+      });
+    });
+  });
+});
@@ -1,7 +1,7 @@
 import { isEqual } from 'lodash';

 import { ScopeDashboardBinding } from '@grafana/data';
-import { config } from '@grafana/runtime';
+import { config, locationService } from '@grafana/runtime';

 import { ScopesApiClient } from '../ScopesApiClient';
 import { ScopesServiceBase } from '../ScopesServiceBase';
@@ -113,6 +113,9 @@ export class ScopesDashboardsService extends ScopesServiceBase<ScopesDashboardsS
  public groupSuggestedItems = (
    navigationItems: Array<ScopeDashboardBinding | ScopeNavigation>
  ): SuggestedNavigationsFoldersMap => {
+    const currentPath = locationService.getLocation().pathname;
+    const isCurrentDashboard = currentPath.startsWith('/d/');
+
    const folders: SuggestedNavigationsFoldersMap = {
      '': {
        title: '',
@@ -127,15 +130,33 @@ export class ScopesDashboardsService extends ScopesServiceBase<ScopesDashboardsS
      const rootNode = folders[''];
      const groups = navigation.status.groups ?? [];

+      // If the current URL matches an item, expand the parent folders.
+      let expanded = false;
+
+      if (isCurrentDashboard && 'dashboard' in navigation.spec) {
+        const dashboardId = currentPath.split('/')[2];
+        expanded = navigation.spec.dashboard === dashboardId;
+      }
+
+      if ('url' in navigation.spec) {
+        expanded = currentPath.startsWith(navigation.spec.url);
+      }
+
      groups.forEach((group) => {
-        if (group && !rootNode.folders[group]) {
+        const groupExists = !!rootNode.folders[group];
+        const groupCurrentlyExpanded = groupExists && rootNode.folders[group].expanded;
+
+        if (group && !groupExists) {
          rootNode.folders[group] = {
            title: group,
-            expanded: false,
+            expanded,
            folders: {},
            suggestedNavigations: {},
          };
        }
+        if (group && expanded && !groupCurrentlyExpanded) {
+          rootNode.folders[group].expanded = true;
+        }
      });

      const targets =
@@ -30,7 +30,7 @@ export function ScopesDashboardsTree({ folders, folderPath, onFolderUpdate }: Sc
      ))}
      {Object.values(folder.suggestedNavigations).map((navigation) => (
        <ScopesNavigationTreeLink
-          key={navigation.id}
+          key={navigation.id + navigation.title}
          to={urlUtil.renderUrl(navigation.url, queryParams)}
          title={navigation.title}
          id={navigation.id}
@@ -70,7 +70,7 @@ const getStyles = (theme: GrafanaTheme2) => {
      marginTop: theme.spacing(0.25),
    }),
    children: css({
-      paddingLeft: theme.spacing(4),
+      paddingLeft: theme.spacing(3),
    }),
  };
 };
@@ -1,6 +1,6 @@
-import { css } from '@emotion/css';
+import { css, cx } from '@emotion/css';
 import { useMemo } from 'react';
-import { Link } from 'react-router-dom-v5-compat';
+import { Link, useLocation } from 'react-router-dom-v5-compat';

 import { GrafanaTheme2, IconName, locationUtil } from '@grafana/data';
 import { Icon, useStyles2 } from '@grafana/ui';
@@ -14,10 +14,22 @@ export interface ScopesNavigationTreeLinkProps {
 export function ScopesNavigationTreeLink({ to, title, id }: ScopesNavigationTreeLinkProps) {
  const styles = useStyles2(getStyles);
  const linkIcon = useMemo(() => getLinkIcon(to), [to]);
+  const isDashboard = to.startsWith('/d/');
+
+  // For dashboards, the title is appended to the path. We need to diregard this
+  const currentPath = isDashboard ? useLocation().pathname.split('/').slice(0, 3).join('/') : useLocation().pathname;
+
+  const isCurrent = to.startsWith(currentPath);

  return (
-    <Link to={to} className={styles.container} data-testid={`scopes-dashboards-${id}`} role="treeitem">
-      <Icon name={linkIcon} className={styles.icon} /> {title}
+    <Link
+      to={to}
+      className={cx(styles.container, isCurrent && styles.current)}
+      data-testid={`scopes-dashboards-${id}`}
+      role="treeitem"
+      key={id}
+    >
+      <Icon name={linkIcon} /> {title}
    </Link>
  );
 }
@@ -48,21 +60,33 @@ const getStyles = (theme: GrafanaTheme2) => {
  return {
    container: css({
      display: 'flex',
-      alignItems: 'flex-start',
+      alignItems: 'center',
      gap: theme.spacing(1),
-      padding: theme.spacing(0.5, 0),
+      padding: theme.spacing(0.75, 0),
      textAlign: 'left',
+      paddingLeft: theme.spacing(1),
+
      wordBreak: 'break-word',

-      '&:last-child': css({
-        paddingBottom: 0,
-      }),
      '&:hover, &:focus': css({
        textDecoration: 'underline',
      }),
    }),
-    icon: css({
-      marginTop: theme.spacing(0.25),
+    current: css({
+      position: 'relative',
+      background: theme.colors.action.selected,
+      borderRadius: `0 ${theme.shape.radius.default} ${theme.shape.radius.default} 0`,
+      '&::before': {
+        backgroundImage: theme.colors.gradients.brandVertical,
+        borderRadius: theme.shape.radius.default,
+        content: '" "',
+        display: 'block',
+        height: '100%',
+        position: 'absolute',
+        width: theme.spacing(0.5),
+        top: 0,
+        left: 0,
+      },
    }),
  };
 };