Use links instead of click handlers when switching spaces (#57730)

* use links instead of click handlers when switching spaces

* remove unused imports

* remove use of deprecated injectedMetadata service

* simplify SpacesManager constructor

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

x-pack/plugins/spaces/common/index.ts

@@ -5,5 +5,5 @@
  */
 
 export { isReservedSpace } from './is_reserved_space';
-export { MAX_SPACE_INITIALS, SPACE_SEARCH_COUNT_THRESHOLD } from './constants';
+export { MAX_SPACE_INITIALS, SPACE_SEARCH_COUNT_THRESHOLD, ENTER_SPACE_PATH } from './constants';
 export { addSpaceIdToPath, getSpaceIdFromPath } from './lib/spaces_url_parser';

x-pack/plugins/spaces/public/nav_control/components/spaces_menu.tsx

@@ -14,7 +14,7 @@ import {
 import { FormattedMessage, InjectedIntl, injectI18n } from '@kbn/i18n/react';
 import React, { Component, ReactElement } from 'react';
 import { Capabilities, ApplicationStart } from 'src/core/public';
-import { SPACE_SEARCH_COUNT_THRESHOLD } from '../../../common/constants';
+import { addSpaceIdToPath, SPACE_SEARCH_COUNT_THRESHOLD, ENTER_SPACE_PATH } from '../../../common';
 import { Space } from '../../../common/model/space';
 import { ManageSpacesButton } from './manage_spaces_button';
 import { SpaceAvatar } from '../../space_avatar';
@@ -23,7 +23,7 @@ interface Props {
   id: string;
   spaces: Space[];
   isLoading: boolean;
-  onSelectSpace: (space: Space) => void;
+  serverBasePath: string;
   onManageSpacesClick: () => void;
   intl: InjectedIntl;
   capabilities: Capabilities;
@@ -184,7 +184,7 @@ class SpacesMenuUI extends Component<Props, State> {
       <EuiContextMenuItem
         key={space.id}
         icon={icon}
-        onClick={this.props.onSelectSpace.bind(this, space)}
+        href={addSpaceIdToPath(this.props.serverBasePath, space.id, ENTER_SPACE_PATH)}
         toolTipTitle={space.description && space.name}
         toolTipContent={space.description}
       >

x-pack/plugins/spaces/public/nav_control/nav_control.tsx

@@ -23,6 +23,7 @@ export function initSpacesNavControl(spacesManager: SpacesManager, core: CoreSta
         <I18nContext>
           <NavControlPopover
             spacesManager={spacesManager}
+            serverBasePath={core.http.basePath.serverBasePath}
             anchorPosition="downLeft"
             capabilities={core.application.capabilities}
             navigateToApp={core.application.navigateToApp}

x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx

@@ -21,6 +21,7 @@ describe('NavControlPopover', () => {
     const wrapper = shallow(
       <NavControlPopover
         spacesManager={(spacesManager as unknown) as SpacesManager}
+        serverBasePath={'/server-base-path'}
         anchorPosition={'downRight'}
         capabilities={{ navLinks: {}, management: {}, catalogue: {}, spaces: { manage: true } }}
         navigateToApp={jest.fn()}
@@ -53,6 +54,7 @@ describe('NavControlPopover', () => {
     const wrapper = mountWithIntl(
       <NavControlPopover
         spacesManager={(spacesManager as unknown) as SpacesManager}
+        serverBasePath={'/server-base-path'}
         anchorPosition={'rightCenter'}
         capabilities={{ navLinks: {}, management: {}, catalogue: {}, spaces: { manage: true } }}
         navigateToApp={jest.fn()}

x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx

@@ -24,6 +24,7 @@ interface Props {
   anchorPosition: PopoverAnchorPosition;
   capabilities: Capabilities;
   navigateToApp: ApplicationStart['navigateToApp'];
+  serverBasePath: string;
 }
 
 interface State {
@@ -86,7 +87,7 @@ export class NavControlPopover extends Component<Props, State> {
           id={popoutContentId}
           spaces={this.state.spaces}
           isLoading={this.state.loading}
-          onSelectSpace={this.onSelectSpace}
+          serverBasePath={this.props.serverBasePath}
           onManageSpacesClick={this.toggleSpaceSelector}
           capabilities={this.props.capabilities}
           navigateToApp={this.props.navigateToApp}
@@ -175,8 +176,4 @@ export class NavControlPopover extends Component<Props, State> {
       showSpaceSelector: false,
     });
   };
-
-  private onSelectSpace = (space: Space) => {
-    this.props.spacesManager.changeSelectedSpace(space);
-  };
 }

x-pack/plugins/spaces/public/plugin.tsx

@@ -43,8 +43,7 @@ export class SpacesPlugin implements Plugin<SpacesPluginSetup, SpacesPluginStart
   private managementService?: ManagementService;
 
   public setup(core: CoreSetup, plugins: PluginsSetup) {
-    const serverBasePath = core.injectedMetadata.getInjectedVar('serverBasePath') as string;
-    this.spacesManager = new SpacesManager(serverBasePath, core.http);
+    this.spacesManager = new SpacesManager(core.http);
 
     if (plugins.home) {
       plugins.home.featureCatalogue.register(createSpacesFeatureCatalogueEntry());

x-pack/plugins/spaces/public/space_selector/components/space_card.test.tsx

@@ -7,6 +7,7 @@
 import { mount, shallow } from 'enzyme';
 import React from 'react';
 import { SpaceCard } from './space_card';
+import { EuiCard } from '@elastic/eui';
 
 test('it renders without crashing', () => {
   const space = {
@@ -16,21 +17,33 @@ test('it renders without crashing', () => {
     disabledFeatures: [],
   };
 
-  shallow(<SpaceCard space={space} onClick={jest.fn()} />);
+  shallow(<SpaceCard space={space} serverBasePath={'/server-base-path'} />);
 });
 
-test('it is clickable', () => {
+test('links to the indicated space', () => {
   const space = {
-    id: '',
+    id: 'some-space',
     name: 'space name',
     description: 'space description',
     disabledFeatures: [],
   };
 
-  const clickHandler = jest.fn();
-
-  const wrapper = mount(<SpaceCard space={space} onClick={clickHandler} />);
-  wrapper.find('button').simulate('click');
-
-  expect(clickHandler).toHaveBeenCalledTimes(1);
+  const wrapper = mount(<SpaceCard space={space} serverBasePath={'/server-base-path'} />);
+  expect(wrapper.find(EuiCard).props()).toMatchObject({
+    href: '/server-base-path/s/some-space/spaces/enter',
+  });
+});
+
+test('links to the default space too', () => {
+  const space = {
+    id: 'default',
+    name: 'default space',
+    description: 'space description',
+    disabledFeatures: [],
+  };
+
+  const wrapper = mount(<SpaceCard space={space} serverBasePath={'/server-base-path'} />);
+  expect(wrapper.find(EuiCard).props()).toMatchObject({
+    href: '/server-base-path/spaces/enter',
+  });
 });

x-pack/plugins/spaces/public/space_selector/components/space_card.tsx

@@ -6,15 +6,16 @@
 
 import { EuiCard } from '@elastic/eui';
 import React from 'react';
-import { Space } from '../../../common/model/space';
+import { addSpaceIdToPath, ENTER_SPACE_PATH } from '../../../common';
 import { SpaceAvatar } from '../../space_avatar';
+import { Space } from '../..';
 
 interface Props {
   space: Space;
-  onClick: () => void;
+  serverBasePath: string;
 }
 export const SpaceCard = (props: Props) => {
-  const { space, onClick } = props;
+  const { serverBasePath, space } = props;
 
   return (
     <EuiCard
@@ -23,7 +24,7 @@ export const SpaceCard = (props: Props) => {
       icon={renderSpaceAvatar(space)}
       title={space.name}
       description={renderSpaceDescription(space)}
-      onClick={onClick}
+      href={addSpaceIdToPath(serverBasePath, space.id, ENTER_SPACE_PATH)}
     />
   );
 };

x-pack/plugins/spaces/public/space_selector/components/space_cards.test.tsx

@@ -16,5 +16,5 @@ test('it renders without crashing', () => {
     disabledFeatures: [],
   };
 
-  shallow(<SpaceCards spaces={[space]} onSpaceSelect={jest.fn()} />);
+  shallow(<SpaceCards spaces={[space]} serverBasePath={'/server-base-path'} />);
 });

x-pack/plugins/spaces/public/space_selector/components/space_cards.tsx

@@ -11,7 +11,7 @@ import { SpaceCard } from './space_card';
 
 interface Props {
   spaces: Space[];
-  onSpaceSelect: (space: Space) => void;
+  serverBasePath: string;
 }
 
 export class SpaceCards extends Component<Props, {}> {
@@ -25,15 +25,9 @@ export class SpaceCards extends Component<Props, {}> {
     );
   }
 
-  public renderSpace = (space: Space) => (
+  private renderSpace = (space: Space) => (
     <EuiFlexItem key={space.id} grow={false}>
-      <SpaceCard space={space} onClick={this.createSpaceClickHandler(space)} />
+      <SpaceCard space={space} serverBasePath={this.props.serverBasePath} />
     </EuiFlexItem>
   );
-
-  public createSpaceClickHandler = (space: Space) => {
-    return () => {
-      this.props.onSpaceSelect(space);
-    };
-  };
 }

x-pack/plugins/spaces/public/space_selector/space_selector.test.tsx

@@ -18,7 +18,9 @@ function getSpacesManager(spaces: Space[] = []) {
 
 test('it renders without crashing', () => {
   const spacesManager = getSpacesManager();
-  const component = shallowWithIntl(<SpaceSelector spacesManager={spacesManager as any} />);
+  const component = shallowWithIntl(
+    <SpaceSelector spacesManager={spacesManager as any} serverBasePath={'/server-base-path'} />
+  );
   expect(component).toMatchSnapshot();
 });
 
@@ -34,7 +36,9 @@ test('it queries for spaces when loaded', () => {
 
   const spacesManager = getSpacesManager(spaces);
 
-  shallowWithIntl(<SpaceSelector spacesManager={spacesManager as any} />);
+  shallowWithIntl(
+    <SpaceSelector spacesManager={spacesManager as any} serverBasePath={'/server-base-path'} />
+  );
 
   return Promise.resolve().then(() => {
     expect(spacesManager.getSpaces).toHaveBeenCalledTimes(1);

x-pack/plugins/spaces/public/space_selector/space_selector.tsx

@@ -30,6 +30,7 @@ import { SpacesManager } from '../spaces_manager';
 
 interface Props {
   spacesManager: SpacesManager;
+  serverBasePath: string;
 }
 
 interface State {
@@ -129,7 +130,7 @@ export class SpaceSelector extends Component<Props, State> {
             {this.state.loading && <EuiLoadingSpinner size="xl" />}
 
             {!this.state.loading && (
-              <SpaceCards spaces={filteredSpaces} onSpaceSelect={this.onSelectSpace} />
+              <SpaceCards spaces={filteredSpaces} serverBasePath={this.props.serverBasePath} />
             )}
 
             {!this.state.loading && filteredSpaces.length === 0 && (
@@ -179,10 +180,6 @@ export class SpaceSelector extends Component<Props, State> {
       searchTerm: searchTerm.trim().toLowerCase(),
     });
   };
-
-  public onSelectSpace = (space: Space) => {
-    this.props.spacesManager.changeSelectedSpace(space);
-  };
 }
 
 export const renderSpaceSelectorApp = (i18nStart: CoreStart['i18n'], el: Element, props: Props) => {

x-pack/plugins/spaces/public/space_selector/space_selector_app.tsx

@@ -29,7 +29,10 @@ export const spaceSelectorApp = Object.freeze({
           getStartServices(),
           import('./space_selector'),
         ]);
-        return renderSpaceSelectorApp(coreStart.i18n, params.element, { spacesManager });
+        return renderSpaceSelectorApp(coreStart.i18n, params.element, {
+          spacesManager,
+          serverBasePath: coreStart.http.basePath.serverBasePath,
+        });
       },
     });
   },

x-pack/plugins/spaces/public/spaces_manager/spaces_manager.mock.ts

@@ -20,7 +20,6 @@ function createSpacesManagerMock() {
     copySavedObjects: jest.fn().mockResolvedValue(undefined),
     resolveCopySavedObjectsErrors: jest.fn().mockResolvedValue(undefined),
     redirectToSpaceSelector: jest.fn().mockResolvedValue(undefined),
-    changeSelectedSpace: jest.fn(),
   } as unknown) as jest.Mocked<SpacesManager>;
 }
 

x-pack/plugins/spaces/public/spaces_manager/spaces_manager.test.ts

@@ -12,14 +12,14 @@ describe('SpacesManager', () => {
   describe('#constructor', () => {
     it('attempts to retrieve the active space', () => {
       const coreStart = coreMock.createStart();
-      new SpacesManager('/server-base-path', coreStart.http);
+      new SpacesManager(coreStart.http);
       expect(coreStart.http.get).toHaveBeenCalledWith('/internal/spaces/_active_space');
     });
 
     it('does not retrieve the active space if on an anonymous path', () => {
       const coreStart = coreMock.createStart();
       coreStart.http.anonymousPaths.isAnonymous.mockReturnValue(true);
-      new SpacesManager('/server-base-path', coreStart.http);
+      new SpacesManager(coreStart.http);
       expect(coreStart.http.get).not.toHaveBeenCalled();
     });
   });
@@ -31,7 +31,7 @@ describe('SpacesManager', () => {
         id: 'my-space',
         name: 'my space',
       });
-      const spacesManager = new SpacesManager('/server-base-path', coreStart.http);
+      const spacesManager = new SpacesManager(coreStart.http);
       expect(coreStart.http.get).toHaveBeenCalledWith('/internal/spaces/_active_space');
 
       await nextTick();
@@ -47,7 +47,7 @@ describe('SpacesManager', () => {
     it('throws if on an anonymous path', () => {
       const coreStart = coreMock.createStart();
       coreStart.http.anonymousPaths.isAnonymous.mockReturnValue(true);
-      const spacesManager = new SpacesManager('/server-base-path', coreStart.http);
+      const spacesManager = new SpacesManager(coreStart.http);
       expect(coreStart.http.get).not.toHaveBeenCalled();
 
       expect(() => spacesManager.getActiveSpace()).toThrowErrorMatchingInlineSnapshot(
@@ -67,7 +67,7 @@ describe('SpacesManager', () => {
           name: 'my other space',
         });
 
-      const spacesManager = new SpacesManager('/server-base-path', coreStart.http);
+      const spacesManager = new SpacesManager(coreStart.http);
       expect(coreStart.http.get).toHaveBeenCalledWith('/internal/spaces/_active_space');
 
       await nextTick();
@@ -95,7 +95,7 @@ describe('SpacesManager', () => {
         name: 'my space',
       });
 
-      const spacesManager = new SpacesManager('/server-base-path', coreStart.http);
+      const spacesManager = new SpacesManager(coreStart.http);
 
       expect(() =>
         spacesManager.getActiveSpace({ forceRefresh: true })

x-pack/plugins/spaces/public/spaces_manager/spaces_manager.ts

@@ -9,16 +9,18 @@ import { HttpSetup } from 'src/core/public';
 import { SavedObjectsManagementRecord } from 'src/legacy/core_plugins/management/public';
 import { Space } from '../../common/model/space';
 import { GetSpacePurpose } from '../../common/model/types';
-import { ENTER_SPACE_PATH } from '../../common/constants';
 import { CopySavedObjectsToSpaceResponse } from '../copy_saved_objects_to_space/types';
-import { addSpaceIdToPath } from '../../common';
 
 export class SpacesManager {
   private activeSpace$: BehaviorSubject<Space | null> = new BehaviorSubject<Space | null>(null);
 
+  private readonly serverBasePath: string;
+
   public readonly onActiveSpaceChange$: Observable<Space>;
 
-  constructor(private readonly serverBasePath: string, private readonly http: HttpSetup) {
+  constructor(private readonly http: HttpSetup) {
+    this.serverBasePath = http.basePath.serverBasePath;
+
     this.onActiveSpaceChange$ = this.activeSpace$
       .asObservable()
       .pipe(skipWhile((v: Space | null) => v == null)) as Observable<Space>;
@@ -99,10 +101,6 @@ export class SpacesManager {
     });
   }
 
-  public async changeSelectedSpace(space: Space) {
-    window.location.href = addSpaceIdToPath(this.serverBasePath, space.id, ENTER_SPACE_PATH);
-  }
-
   public redirectToSpaceSelector() {
     window.location.href = `${this.serverBasePath}/spaces/space_selector`;
   }