mirror of
https://github.com/OpenHands/OpenHands.git
synced 2025-12-26 05:48:36 +08:00
refactor(frontend): consolidate settings navigation items logic into shared custom hook (#11950)
Co-authored-by: amanape <83104063+amanape@users.noreply.github.com>
This commit is contained in:
parent
8a202b945b
commit
6917d45d3a
53
frontend/__tests__/hooks/use-settings-nav-items.test.tsx
Normal file
53
frontend/__tests__/hooks/use-settings-nav-items.test.tsx
Normal file
@ -0,0 +1,53 @@
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { renderHook, waitFor } from "@testing-library/react";
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { SAAS_NAV_ITEMS, OSS_NAV_ITEMS } from "#/constants/settings-nav";
|
||||
import OptionService from "#/api/option-service/option-service.api";
|
||||
import { useSettingsNavItems } from "#/hooks/use-settings-nav-items";
|
||||
|
||||
const queryClient = new QueryClient();
|
||||
const wrapper = ({ children }: { children: React.ReactNode }) => (
|
||||
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
|
||||
);
|
||||
|
||||
const mockConfig = (appMode: "saas" | "oss", hideLlmSettings = false) => {
|
||||
vi.spyOn(OptionService, "getConfig").mockResolvedValue({
|
||||
APP_MODE: appMode,
|
||||
FEATURE_FLAGS: { HIDE_LLM_SETTINGS: hideLlmSettings },
|
||||
} as Awaited<ReturnType<typeof OptionService.getConfig>>);
|
||||
};
|
||||
|
||||
describe("useSettingsNavItems", () => {
|
||||
beforeEach(() => {
|
||||
queryClient.clear();
|
||||
});
|
||||
|
||||
it("should return SAAS_NAV_ITEMS when APP_MODE is 'saas'", async () => {
|
||||
mockConfig("saas");
|
||||
const { result } = renderHook(() => useSettingsNavItems(), { wrapper });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current).toEqual(SAAS_NAV_ITEMS);
|
||||
});
|
||||
});
|
||||
|
||||
it("should return OSS_NAV_ITEMS when APP_MODE is 'oss'", async () => {
|
||||
mockConfig("oss");
|
||||
const { result } = renderHook(() => useSettingsNavItems(), { wrapper });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current).toEqual(OSS_NAV_ITEMS);
|
||||
});
|
||||
});
|
||||
|
||||
it("should filter out '/settings' item when HIDE_LLM_SETTINGS feature flag is enabled", async () => {
|
||||
mockConfig("saas", true);
|
||||
const { result } = renderHook(() => useSettingsNavItems(), { wrapper });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(
|
||||
result.current.find((item) => item.to === "/settings"),
|
||||
).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -5,11 +5,10 @@ import { ContextMenu } from "#/ui/context-menu";
|
||||
import { ContextMenuListItem } from "./context-menu-list-item";
|
||||
import { Divider } from "#/ui/divider";
|
||||
import { useClickOutsideElement } from "#/hooks/use-click-outside-element";
|
||||
import { useConfig } from "#/hooks/query/use-config";
|
||||
import { I18nKey } from "#/i18n/declaration";
|
||||
import LogOutIcon from "#/icons/log-out.svg?react";
|
||||
import DocumentIcon from "#/icons/document.svg?react";
|
||||
import { SAAS_NAV_ITEMS, OSS_NAV_ITEMS } from "#/constants/settings-nav";
|
||||
import { useSettingsNavItems } from "#/hooks/use-settings-nav-items";
|
||||
|
||||
interface AccountSettingsContextMenuProps {
|
||||
onLogout: () => void;
|
||||
@ -22,15 +21,8 @@ export function AccountSettingsContextMenu({
|
||||
}: AccountSettingsContextMenuProps) {
|
||||
const ref = useClickOutsideElement<HTMLUListElement>(onClose);
|
||||
const { t } = useTranslation();
|
||||
const { data: config } = useConfig();
|
||||
|
||||
const isSaas = config?.APP_MODE === "saas";
|
||||
|
||||
// Get navigation items and filter out LLM settings if the feature flag is enabled
|
||||
let items = isSaas ? SAAS_NAV_ITEMS : OSS_NAV_ITEMS;
|
||||
if (config?.FEATURE_FLAGS?.HIDE_LLM_SETTINGS) {
|
||||
items = items.filter((item) => item.to !== "/settings");
|
||||
}
|
||||
const items = useSettingsNavItems();
|
||||
|
||||
const navItems = items.map((item) => ({
|
||||
...item,
|
||||
@ -39,11 +31,7 @@ export function AccountSettingsContextMenu({
|
||||
height: 16,
|
||||
} as React.SVGProps<SVGSVGElement>),
|
||||
}));
|
||||
|
||||
const handleNavigationClick = () => {
|
||||
onClose();
|
||||
// The Link component will handle the actual navigation
|
||||
};
|
||||
const handleNavigationClick = () => onClose();
|
||||
|
||||
return (
|
||||
<ContextMenu
|
||||
@ -55,7 +43,7 @@ export function AccountSettingsContextMenu({
|
||||
{navItems.map(({ to, text, icon }) => (
|
||||
<Link key={to} to={to} className="text-decoration-none">
|
||||
<ContextMenuListItem
|
||||
onClick={() => handleNavigationClick()}
|
||||
onClick={handleNavigationClick}
|
||||
className="flex items-center gap-2 p-2 hover:bg-[#5C5D62] rounded h-[30px]"
|
||||
>
|
||||
{icon}
|
||||
|
||||
@ -1,16 +1,11 @@
|
||||
import { useState } from "react";
|
||||
import { MobileHeader } from "./mobile-header";
|
||||
import { SettingsNavigation } from "./settings-navigation";
|
||||
|
||||
interface NavigationItem {
|
||||
to: string;
|
||||
icon: React.ReactNode;
|
||||
text: string;
|
||||
}
|
||||
import { SettingsNavItem } from "#/constants/settings-nav";
|
||||
|
||||
interface SettingsLayoutProps {
|
||||
children: React.ReactNode;
|
||||
navigationItems: NavigationItem[];
|
||||
navigationItems: SettingsNavItem[];
|
||||
}
|
||||
|
||||
export function SettingsLayout({
|
||||
@ -19,13 +14,8 @@ export function SettingsLayout({
|
||||
}: SettingsLayoutProps) {
|
||||
const [isMobileMenuOpen, setIsMobileMenuOpen] = useState(false);
|
||||
|
||||
const toggleMobileMenu = () => {
|
||||
setIsMobileMenuOpen(!isMobileMenuOpen);
|
||||
};
|
||||
|
||||
const closeMobileMenu = () => {
|
||||
setIsMobileMenuOpen(false);
|
||||
};
|
||||
const toggleMobileMenu = () => setIsMobileMenuOpen(!isMobileMenuOpen);
|
||||
const closeMobileMenu = () => setIsMobileMenuOpen(false);
|
||||
|
||||
return (
|
||||
<div className="flex flex-col h-full px-[14px] pt-8">
|
||||
@ -34,7 +24,6 @@ export function SettingsLayout({
|
||||
isMobileMenuOpen={isMobileMenuOpen}
|
||||
onToggleMenu={toggleMobileMenu}
|
||||
/>
|
||||
|
||||
{/* Desktop layout with navigation and main content */}
|
||||
<div className="flex flex-1 overflow-hidden gap-10">
|
||||
{/* Navigation */}
|
||||
@ -43,7 +32,6 @@ export function SettingsLayout({
|
||||
onCloseMobileMenu={closeMobileMenu}
|
||||
navigationItems={navigationItems}
|
||||
/>
|
||||
|
||||
{/* Main content */}
|
||||
<main className="flex-1 overflow-auto custom-scrollbar-always">
|
||||
{children}
|
||||
|
||||
@ -5,17 +5,12 @@ import { Typography } from "#/ui/typography";
|
||||
import { I18nKey } from "#/i18n/declaration";
|
||||
import SettingsIcon from "#/icons/settings-gear.svg?react";
|
||||
import CloseIcon from "#/icons/close.svg?react";
|
||||
|
||||
interface NavigationItem {
|
||||
to: string;
|
||||
icon: React.ReactNode;
|
||||
text: string;
|
||||
}
|
||||
import { SettingsNavItem } from "#/constants/settings-nav";
|
||||
|
||||
interface SettingsNavigationProps {
|
||||
isMobileMenuOpen: boolean;
|
||||
onCloseMobileMenu: () => void;
|
||||
navigationItems: NavigationItem[];
|
||||
navigationItems: SettingsNavItem[];
|
||||
}
|
||||
|
||||
export function SettingsNavigation({
|
||||
@ -34,7 +29,6 @@ export function SettingsNavigation({
|
||||
onClick={onCloseMobileMenu}
|
||||
/>
|
||||
)}
|
||||
|
||||
{/* Navigation sidebar */}
|
||||
<nav
|
||||
data-testid="settings-navbar"
|
||||
|
||||
15
frontend/src/hooks/use-settings-nav-items.ts
Normal file
15
frontend/src/hooks/use-settings-nav-items.ts
Normal file
@ -0,0 +1,15 @@
|
||||
import { useConfig } from "#/hooks/query/use-config";
|
||||
import { SAAS_NAV_ITEMS, OSS_NAV_ITEMS } from "#/constants/settings-nav";
|
||||
|
||||
export function useSettingsNavItems() {
|
||||
const { data: config } = useConfig();
|
||||
|
||||
const shouldHideLlmSettings = !!config?.FEATURE_FLAGS?.HIDE_LLM_SETTINGS;
|
||||
const isSaasMode = config?.APP_MODE === "saas";
|
||||
|
||||
const items = isSaasMode ? SAAS_NAV_ITEMS : OSS_NAV_ITEMS;
|
||||
|
||||
return shouldHideLlmSettings
|
||||
? items.filter((item) => item.to !== "/settings")
|
||||
: items;
|
||||
}
|
||||
@ -1,14 +1,13 @@
|
||||
import { useMemo } from "react";
|
||||
import { Outlet, redirect, useLocation } from "react-router";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useConfig } from "#/hooks/query/use-config";
|
||||
import { Route } from "./+types/settings";
|
||||
import OptionService from "#/api/option-service/option-service.api";
|
||||
import { queryClient } from "#/query-client-config";
|
||||
import { GetConfigResponse } from "#/api/option-service/option.types";
|
||||
import { SAAS_NAV_ITEMS, OSS_NAV_ITEMS } from "#/constants/settings-nav";
|
||||
import { Typography } from "#/ui/typography";
|
||||
import { SettingsLayout } from "#/components/features/settings/settings-layout";
|
||||
import { Typography } from "#/ui/typography";
|
||||
import { useSettingsNavItems } from "#/hooks/use-settings-nav-items";
|
||||
|
||||
const SAAS_ONLY_PATHS = [
|
||||
"/settings/user",
|
||||
@ -33,14 +32,10 @@ export const clientLoader = async ({ request }: Route.ClientLoaderArgs) => {
|
||||
// if in OSS mode, do not allow access to saas-only paths
|
||||
return redirect("/settings");
|
||||
}
|
||||
|
||||
// If LLM settings are hidden and user tries to access the LLM settings page
|
||||
if (config?.FEATURE_FLAGS?.HIDE_LLM_SETTINGS && pathname === "/settings") {
|
||||
// Redirect to the first available settings page
|
||||
if (isSaas) {
|
||||
return redirect("/settings/user");
|
||||
}
|
||||
return redirect("/settings/mcp");
|
||||
return isSaas ? redirect("/settings/user") : redirect("/settings/mcp");
|
||||
}
|
||||
|
||||
return null;
|
||||
@ -48,37 +43,15 @@ export const clientLoader = async ({ request }: Route.ClientLoaderArgs) => {
|
||||
|
||||
function SettingsScreen() {
|
||||
const { t } = useTranslation();
|
||||
const { data: config } = useConfig();
|
||||
const location = useLocation();
|
||||
|
||||
const isSaas = config?.APP_MODE === "saas";
|
||||
|
||||
// Navigation items configuration
|
||||
const navItems = useMemo(() => {
|
||||
const items = [];
|
||||
if (isSaas) {
|
||||
items.push(...SAAS_NAV_ITEMS);
|
||||
} else {
|
||||
items.push(...OSS_NAV_ITEMS);
|
||||
}
|
||||
|
||||
// Filter out LLM settings if the feature flag is enabled
|
||||
if (config?.FEATURE_FLAGS?.HIDE_LLM_SETTINGS) {
|
||||
return items.filter((item) => item.to !== "/settings");
|
||||
}
|
||||
|
||||
return items;
|
||||
}, [isSaas, config?.FEATURE_FLAGS?.HIDE_LLM_SETTINGS]);
|
||||
|
||||
const navItems = useSettingsNavItems();
|
||||
// Current section title for the main content area
|
||||
const currentSectionTitle = useMemo(() => {
|
||||
const currentItem = navItems.find((item) => item.to === location.pathname);
|
||||
if (currentItem) {
|
||||
return currentItem.text;
|
||||
}
|
||||
|
||||
// Default to the first available navigation item if current page is not found
|
||||
return navItems.length > 0 ? navItems[0].text : "SETTINGS$TITLE";
|
||||
return currentItem
|
||||
? currentItem.text
|
||||
: (navItems[0]?.text ?? "SETTINGS$TITLE");
|
||||
}, [navItems, location.pathname]);
|
||||
|
||||
return (
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user