Skip to content

Commit

Permalink
feat(ws): Notebooks 2.0 // Frontend // Namespace selector
Browse files Browse the repository at this point in the history
Signed-off-by: yelias <[email protected]>
  • Loading branch information
yelias committed Jan 7, 2025
1 parent ca76781 commit 60ee75b
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 37 deletions.
8 changes: 5 additions & 3 deletions workspaces/frontend/src/__mocks__/mockNamespaces.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { NamespacesList } from '~/app/types';

export const mockNamespaces = (): NamespacesList => ({
data: [{ name: 'default' }, { name: 'kubeflow' }, { name: 'custom-namespace' }],
});
export const mockNamespaces: NamespacesList = [
{ name: 'default' },
{ name: 'kubeflow' },
{ name: 'custom-namespace' },
];
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import { mockNamespaces } from '~/__mocks__/mockNamespaces';
import { mockBFFResponse } from '~/__mocks__/utils';

const namespaces = ['default', 'kubeflow', 'custom-namespace'];
const mockNamespaces = {
data: [{ name: 'default' }, { name: 'kubeflow' }, { name: 'custom-namespace' }],
};

describe('Namespace Selector Dropdown', () => {
beforeEach(() => {
// Mock the namespaces and selected namespace
// Mock the namespaces API response
cy.intercept('GET', '/api/v1/namespaces', {
body: mockNamespaces,
});
body: mockBFFResponse(mockNamespaces),
}).as('getNamespaces');
cy.visit('/');
cy.wait('@getNamespaces');
});

it('should open the namespace dropdown and select a namespace', () => {
Expand Down Expand Up @@ -38,4 +39,19 @@ describe('Namespace Selector Dropdown', () => {
cy.get('[data-testid="nav-link-/notebookSettings"]').click();
cy.get('[data-testid="namespace-toggle"]').should('contain', 'custom-namespace');
});

it('should filter namespaces based on search input', () => {
cy.get('[data-testid="namespace-toggle"]').click();
cy.get('[data-testid="namespace-search-input"]').type('custom');
cy.get('[data-testid="namespace-search-input"] input').should('have.value', 'custom');
cy.get('[data-testid="namespace-search-button"]').click();
// Verify that only the matching namespace is displayed
namespaces.forEach((ns) => {
if (ns === 'custom-namespace') {
cy.get(`[data-testid="dropdown-item-${ns}"]`).should('exist').and('contain', ns);
} else {
cy.get(`[data-testid="dropdown-item-${ns}"]`).should('not.exist');
}
});
});
});
Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
import { pageNotfound } from '~/__tests__/cypress/cypress/pages/pageNotFound';
import { home } from '~/__tests__/cypress/cypress/pages/home';
import { mockNamespaces } from '~/__mocks__/mockNamespaces';
import { mockBFFResponse } from '~/__mocks__/utils';

describe('Application', () => {
beforeEach(() => {
// Mock the namespaces API response
cy.intercept('GET', '/api/v1/namespaces', {
body: mockBFFResponse(mockNamespaces),
}).as('getNamespaces');
cy.visit('/');
cy.wait('@getNamespaces');
});

it('Page not found should render', () => {
pageNotfound.visit();
});
Expand Down
2 changes: 1 addition & 1 deletion workspaces/frontend/src/app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const App: React.FC = () => {
return (
<NotebookContextProvider>
<NamespaceProvider>
<Page
<Page
mainContainerId="primary-app-container"
masthead={masthead}
isManagedSidebar
Expand Down
27 changes: 18 additions & 9 deletions workspaces/frontend/src/app/context/NamespaceContextProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React, { useState, useContext, ReactNode, useMemo, useCallback } from 'react';
import useMount from '~/app/hooks/useMount';
import useNamespaces from '~/app/hooks/useNamespaces';

interface NamespaceContextState {
namespaces: string[];
Expand All @@ -24,21 +25,29 @@ interface NamespaceProviderProps {
export const NamespaceProvider: React.FC<NamespaceProviderProps> = ({ children }) => {
const [namespaces, setNamespaces] = useState<string[]>([]);
const [selectedNamespace, setSelectedNamespace] = useState<string>('');
const [namespacesData, loaded, loadError] = useNamespaces();

const fetchNamespaces = useCallback(() => {
if (loaded && namespacesData) {
const namespaceNames = namespacesData.map((ns) => ns.name);
setNamespaces(namespaceNames);
setSelectedNamespace(namespaceNames.length > 0 ? namespaceNames[0] : '');
} else {
if (loadError) {
console.error('Error loading namespaces: ', loadError);
}
setNamespaces([]);
setSelectedNamespace('');
}
}, [loaded, namespacesData, loadError]);

// Todo: Need to replace with actual API call
const fetchNamespaces = useCallback(async () => {
const mockNamespaces = {
data: [{ name: 'default' }, { name: 'kubeflow' }, { name: 'custom-namespace' }],
};
const namespaceNames = mockNamespaces.data.map((ns) => ns.name);
setNamespaces(namespaceNames);
setSelectedNamespace(namespaceNames.length > 0 ? namespaceNames[0] : '');
}, []);
useMount(fetchNamespaces);

const namespacesContextValues = useMemo(
() => ({ namespaces, selectedNamespace, setSelectedNamespace }),
[namespaces, selectedNamespace],
);

return (
<NamespaceContext.Provider value={namespacesContextValues}>
{children}
Expand Down
2 changes: 1 addition & 1 deletion workspaces/frontend/src/app/context/NotebookContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const NotebookContext = React.createContext<NotebookContextType>({
});

export const NotebookContextProvider: React.FC = ({ children }) => {
const hostPath = `/api/${BFF_API_VERSION}/`;
const hostPath = `/api/${BFF_API_VERSION}`;

const [apiState, refreshAPIState] = useNotebookAPIState(hostPath);

Expand Down
2 changes: 1 addition & 1 deletion workspaces/frontend/src/app/hooks/useMount.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useEffect } from 'react';
const useMount = (callback: () => void): void => {
useEffect(() => {
callback();
}, [callback]);
}, []);

Check failure on line 6 in workspaces/frontend/src/app/hooks/useMount.tsx

View workflow job for this annotation

GitHub Actions / test-and-build

React Hook useEffect has a missing dependency: 'callback'. Either include it or remove the dependency array. If 'callback' changes too often, find the parent component that defines it and wrap that definition in useCallback
};

export default useMount;
4 changes: 1 addition & 3 deletions workspaces/frontend/src/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ export type Namespace = {
name: string;
};

export type NamespacesList = {
data: Namespace[];
};
export type NamespacesList = Namespace[];

export type GetNamespaces = (opts: APIOptions) => Promise<NamespacesList>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ const APIOptionsMock = {};

describe('getNamespaces', () => {
it('should call restGET and handleRestFailures to fetch namespaces', async () => {
const response = await getNamespaces(`/api/${BFF_API_VERSION}/namespaces/`)(APIOptionsMock);
const response = await getNamespaces(`/api/${BFF_API_VERSION}/namespaces`)(APIOptionsMock);
expect(response).toEqual(mockRestResponse);
expect(restGETMock).toHaveBeenCalledTimes(1);
expect(restGETMock).toHaveBeenCalledWith(
`/api/${BFF_API_VERSION}/namespaces/`,
`/namespaces/`,
`/api/${BFF_API_VERSION}/namespaces`,
`/namespaces`,
{},
APIOptionsMock,
);
Expand Down
2 changes: 1 addition & 1 deletion workspaces/frontend/src/shared/api/notebookService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { handleRestFailures } from '~/shared/api/errorUtils';
export const getNamespaces =
(hostPath: string) =>
(opts: APIOptions): Promise<NamespacesList> =>
handleRestFailures(restGET(hostPath, `/namespaces/`, {}, opts)).then((response) => {
handleRestFailures(restGET(hostPath, `/namespaces`, {}, opts)).then((response) => {
if (isNotebookResponse<NamespacesList>(response)) {
return response.data;
}
Expand Down
23 changes: 14 additions & 9 deletions workspaces/frontend/src/shared/components/NamespaceSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { useNamespaceContext } from '~/app/context/NamespaceContextProvider';

const NamespaceSelector: FC = () => {
const { namespaces, selectedNamespace, setSelectedNamespace } = useNamespaceContext();
const [isOpen, setIsOpen] = useState<boolean>(false);
const [isNamespaceDropdownOpen, setIsNamespaceDropdownOpen] = useState<boolean>(false);
const [searchInputValue, setSearchInputValue] = useState<string>('');
const [filteredNamespaces, setFilteredNamespaces] = useState<string[]>(namespaces);

Expand All @@ -28,10 +28,10 @@ const NamespaceSelector: FC = () => {
}, [namespaces]);

const onToggleClick = () => {
if (!isOpen) {
if (!isNamespaceDropdownOpen) {
onClearSearch();
}
setIsOpen(!isOpen);
setIsNamespaceDropdownOpen(!isNamespaceDropdownOpen);
};

const onSearchInputChange = (value: string) => {
Expand All @@ -54,10 +54,12 @@ const NamespaceSelector: FC = () => {

const onSelect: DropdownProps['onSelect'] = (_event, value) => {
setSelectedNamespace(value as string);
setIsOpen(false);
setIsNamespaceDropdownOpen(false);
};

const onClearSearch = () => {
const onClearSearch = (event?: React.MouseEvent | React.ChangeEvent | React.FormEvent) => {
// Prevent the event from bubbling up and triggering dropdown close
event?.stopPropagation();
setSearchInputValue('');
setFilteredNamespaces(namespaces);
};
Expand All @@ -84,14 +86,16 @@ const NamespaceSelector: FC = () => {
<MenuToggle
ref={toggleRef}
onClick={onToggleClick}
isExpanded={isOpen}
isExpanded={isNamespaceDropdownOpen}
className="namespace-select-toggle"
data-testid="namespace-toggle"
>
{selectedNamespace}
</MenuToggle>
)}
isOpen={isOpen}
isOpen={isNamespaceDropdownOpen}
onOpenChange={(isOpen) => setIsNamespaceDropdownOpen(isOpen)}
onOpenChangeKeys={['Escape']}
isScrollable
data-testid="namespace-dropdown"
>
Expand All @@ -104,9 +108,9 @@ const NamespaceSelector: FC = () => {
placeholder="Search Namespace"
onChange={(_event, value) => onSearchInputChange(value)}
onKeyDown={onEnterPressed}
onClear={onClearSearch}
// resetButtonLabel="Clear search"
onClear={(event) => onClearSearch(event)}
aria-labelledby="namespace-search-button"
data-testid="namespace-search-input"
/>
</InputGroupItem>
<InputGroupItem>
Expand All @@ -116,6 +120,7 @@ const NamespaceSelector: FC = () => {
id="namespace-search-button"
onClick={onSearchButtonClick}
icon={<SearchIcon aria-hidden="true" />}
data-testid="namespace-search-button"
/>
</InputGroupItem>
</InputGroup>
Expand Down

0 comments on commit 60ee75b

Please sign in to comment.