Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions apps/admin/src/ember-bridge/ember-bridge.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -416,16 +416,19 @@ describe('useEmberRouting', () => {

baseTest('returns bridge routing methods when bridge is available', () => {
const mock = createMockStateBridge();
mock.stateBridge.isRouteActive = vi.fn(() => true);
window.EmberBridge = { state: mock.stateBridge };

const { result } = renderHook(() => useEmberRouting());

expect(result.current).toHaveProperty('getRouteUrl');
expect(result.current).toHaveProperty('isRouteActive');

// Should be using bridge methods, not defaults
// Should be using bridge methods, not defaults. The active-state method is
// wrapped so the app can ignore stale Ember state on React-owned routes.
expect(result.current.getRouteUrl).toBe(mock.stateBridge.getRouteUrl);
expect(result.current.isRouteActive).toBe(mock.stateBridge.isRouteActive);
expect(result.current.isRouteActive('posts')).toBe(true);
expect(mock.stateBridge.isRouteActive).toHaveBeenCalledWith('posts');
});

baseTest('switches to bridge methods when bridge becomes available', async () => {
Expand All @@ -439,6 +442,7 @@ describe('useEmberRouting', () => {

// Bridge becomes available
const mock = createMockStateBridge();
mock.stateBridge.isRouteActive = vi.fn(() => true);
window.EmberBridge = { state: mock.stateBridge };

// Wait for the subscription interval to fire
Expand All @@ -448,7 +452,8 @@ describe('useEmberRouting', () => {

// Now should be using bridge methods
expect(result.current.getRouteUrl).toBe(mock.stateBridge.getRouteUrl);
expect(result.current.isRouteActive).toBe(mock.stateBridge.isRouteActive);
expect(result.current.isRouteActive('posts')).toBe(true);
expect(mock.stateBridge.isRouteActive).toHaveBeenCalledWith('posts');
});

baseTest('re-renders when route changes', async () => {
Expand Down
11 changes: 9 additions & 2 deletions apps/admin/src/ember-bridge/ember-bridge.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useCallback, useEffect, useState, useSyncExternalStore } from 'react';
import { useCallback, useContext, useEffect, useState, useSyncExternalStore } from 'react';
import { useQueryClient } from '@tanstack/react-query';
import { useBrowseConfig } from '@tryghost/admin-x-framework/api/config';
import { EmberContext } from './ember-context';

export interface EmberBridge {
state: StateBridge;
Expand Down Expand Up @@ -360,6 +361,7 @@ const defaultRouting: EmberRouting = {
* ```
*/
export function useEmberRouting(): EmberRouting {
const emberContext = useContext(EmberContext);
const [bridge, setBridge] = useState<StateBridge | null>(() => window.EmberBridge?.state ?? null);
const [, forceUpdate] = useState(0);

Expand All @@ -385,7 +387,12 @@ export function useEmberRouting(): EmberRouting {

return {
getRouteUrl: bridge.getRouteUrl,
isRouteActive: bridge.isRouteActive,
// React-owned navigations use pushState, which Ember does not observe.
// Only trust Ember's route state while the current route is actually
// rendering an Ember fallback. Outside EmberProvider (mainly unit tests
// and standalone consumers), preserve the bridge's original behaviour.
isRouteActive: (...args) =>
(emberContext?.isFallbackPresent ?? true) && bridge.isRouteActive(...args),
};
}

Expand Down
1 change: 1 addition & 0 deletions apps/admin/src/ember-bridge/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ export type {
EmberDataChangeEvent,
EmberRouting,
OpenGiftLinkModalEvent,
StateBridge,
} from './ember-bridge';
14 changes: 11 additions & 3 deletions apps/admin/src/layout/app-sidebar/nav-main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
SidebarMenu,
SidebarMenuBadge,
} from '@tryghost/shade/components';
import { LucideIcon } from '@tryghost/shade/utils';
import { formatNumber, LucideIcon } from '@tryghost/shade/utils';
import { useBrowseSite } from '@tryghost/admin-x-framework/api/site';
import { useCurrentUser } from '@tryghost/admin-x-framework/api/current-user';
import { useBrowseSettings } from '@tryghost/admin-x-framework/api/settings';
Expand Down Expand Up @@ -34,6 +34,11 @@ function NavMain({ ...props }: React.ComponentProps<typeof SidebarGroup>) {
);
const isNetworkRouteActive = useIsActiveLink({ path: 'network', activeOnSubpath: true });
const isActivitypubRouteActive = useIsActiveLink({ path: 'activitypub', activeOnSubpath: true });
const isAnalyticsRouteActive = useIsActiveLink({ path: 'analytics', activeOnSubpath: true });
const isPostAnalyticsRouteActive = useIsActiveLink({
path: 'posts/analytics',
activeOnSubpath: true,
});
const showNetworkBadge =
networkNotificationCount > 0 && !isNetworkRouteActive && !isActivitypubRouteActive;

Expand All @@ -46,7 +51,10 @@ function NavMain({ ...props }: React.ComponentProps<typeof SidebarGroup>) {
<SidebarGroupContent>
<SidebarMenu>
<NavMenuItem>
<NavMenuItem.Link to="analytics" activeOnSubpath>
<NavMenuItem.Link
isActive={isAnalyticsRouteActive || isPostAnalyticsRouteActive}
to="analytics"
>
<LucideIcon.TrendingUp />
<NavMenuItem.Label>Analytics</NavMenuItem.Label>
</NavMenuItem.Link>
Expand All @@ -62,7 +70,7 @@ function NavMain({ ...props }: React.ComponentProps<typeof SidebarGroup>) {
</NavMenuItem.Link>
{showNetworkBadge && (
<SidebarMenuBadge data-testid="network-notification-badge">
{networkNotificationCount}
{formatNumber(networkNotificationCount)}
</SidebarMenuBadge>
)}
</NavMenuItem>
Expand Down
45 changes: 44 additions & 1 deletion apps/admin/src/layout/sidebar.acceptance.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { describe, expect, it } from 'vitest';
import { afterEach, describe, expect, it } from 'vitest';
import type { StateBridge } from '@/ember-bridge';

import {
activeThemeResponse,
Expand Down Expand Up @@ -30,6 +31,29 @@ function fakeUnreadNotifications(count: number): void {
fakeEndpoint('GET', UNREAD_COUNT_URL, { count });
}

function installStaleEmberRoute(activeRoute: 'members-activity' | 'pages' | 'posts'): void {
window.EmberBridge = {
state: {
onUpdate: () => {},
onInvalidate: () => {},
onDelete: () => {},
isFeatureEnabled: () => false,
on: () => {},
off: () => {},
sidebarVisible: true,
getRouteUrl: (routeName) => routeName,
isRouteActive: (routeNames) => {
const routes = Array.isArray(routeNames) ? routeNames : routeNames.split(' ');
return routes.includes(activeRoute);
},
} satisfies StateBridge,
};
}

afterEach(() => {
delete window.EmberBridge;
});

describe('Sidebar navigation', () => {
it('renders the navigation for the current user', async () => {
await renderAdminApp('/site');
Expand Down Expand Up @@ -89,6 +113,25 @@ describe('Sidebar navigation', () => {
await expect.poll(currentRoute).toBe('/pages');
});

it.each([
{ label: 'Posts', route: '/posts', emberRoute: 'posts' },
{ label: 'Pages', route: '/pages', emberRoute: 'pages' },
{ label: 'Members', route: '/members-activity', emberRoute: 'members-activity' },
] as const)(
'clears the $label active state after leaving its Ember route',
async ({ label, route, emberRoute }) => {
fakeTags([]);
installStaleEmberRoute(emberRoute);
await renderAdminApp(route);

await expect.element(sidebarScreen.navLink(label)).toHaveAttribute('aria-current', 'page');

await sidebarScreen.navLink('Tags').click();
await expect.poll(currentRoute).toBe('/tags');
await expect.element(sidebarScreen.navLink(label)).not.toHaveAttribute('aria-current');
},
);

it('shows the default post views and collapses them with the toggle', async () => {
await renderAdminApp('/posts');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
webAnalyticsBootOverrides,
} from '@test-utils/acceptance';
import { membersScreen } from '@/members/members.screen';
import { sidebarScreen } from '@/layout/sidebar.screen';
import { postAnalyticsScreen } from './post-analytics.screen';

const POST_ID = '64d623b64676110001e897d9';
Expand Down Expand Up @@ -133,6 +134,10 @@ describe('Post analytics overview', () => {

await expect.element(postAnalyticsScreen.postTitle('Attack of the Clones')).toBeVisible();
await expect(postsApi).toHaveSentFilter(`id:${POST_ID}`);
await expect
.element(sidebarScreen.navLink('Analytics'))
.toHaveAttribute('aria-current', 'page');
await expect.element(sidebarScreen.navLink('Posts')).not.toHaveAttribute('aria-current');

// Web performance: visitors summed from the Tinybird rows.
await expect.element(postAnalyticsScreen.webPerformanceCard()).toBeVisible();
Expand Down
Loading