Skip to content
Closed
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
22 changes: 10 additions & 12 deletions apps/mail/components/mail/mail-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -603,18 +603,16 @@ const Thread = memo(
</div>
) : null;

return latestMessage ? (
!optimisticState.shouldHide && idToUse ? (
<ThreadContextMenu
threadId={idToUse}
isInbox={isFolderInbox}
isSpam={isFolderSpam}
isSent={isFolderSent}
isBin={isFolderBin}
>
{content}
</ThreadContextMenu>
) : null
return latestMessage && idToUse ? (
<ThreadContextMenu
threadId={idToUse}
isInbox={isFolderInbox}
isSpam={isFolderSpam}
isSent={isFolderSent}
isBin={isFolderBin}
>
{content}
</ThreadContextMenu>
) : null;
},
(prev, next) => {
Expand Down
4 changes: 2 additions & 2 deletions apps/mail/components/ui/app-sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import { useBilling } from '@/hooks/use-billing';
import { useIsMobile } from '@/hooks/use-mobile';
import { Button } from '@/components/ui/button';
import { useAIFullScreen } from './ai-sidebar';
import { useStats } from '@/hooks/use-stats';
import { useAdjustedStats } from '@/hooks/use-stats';
import { useLocation } from 'react-router';
import { useForm } from 'react-hook-form';
import { m } from '@/paraglide/messages';
Expand All @@ -63,7 +63,7 @@ export function AppSidebar({ ...props }: React.ComponentProps<typeof Sidebar>) {

const { isFullScreen } = useAIFullScreen();

const { data: stats } = useStats();
const { data: stats } = useAdjustedStats();

const location = useLocation();
const { data: session, isPending: isSessionPending } = useSession();
Expand Down
8 changes: 4 additions & 4 deletions apps/mail/components/ui/nav-main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { m } from '../../paraglide/messages.js';
import { Button } from '@/components/ui/button';
import { useLabels } from '@/hooks/use-labels';
import { Badge } from '@/components/ui/badge';
import { useStats } from '@/hooks/use-stats';
import { useAdjustedStats } from '@/hooks/use-stats';
import SidebarLabels from './sidebar-labels';
import { useCallback, useRef } from 'react';
import { BASE_URL } from '@/lib/constants';
Expand Down Expand Up @@ -56,7 +56,7 @@ export function NavMain({ items }: NavMainProps) {
const searchParams = new URLSearchParams();
const [category] = useQueryState('category');
const { data: connections } = useConnections();
const { data: stats } = useStats();
const { data: stats } = useAdjustedStats();
const { data: activeConnection } = useActiveConnection();
const trpc = useTRPC();
const { data: intercomToken } = useQuery(trpc.user.getIntercomToken.queryOptions());
Expand Down Expand Up @@ -260,7 +260,7 @@ export function NavMain({ items }: NavMainProps) {
</div>

{activeAccount ? (
<SidebarLabels data={data ?? []} activeAccount={activeAccount} stats={stats} />
<SidebarLabels data={data ?? []} activeAccount={activeAccount} />
) : null}
</SidebarMenuItem>
</Collapsible>
Expand All @@ -272,7 +272,7 @@ export function NavMain({ items }: NavMainProps) {

function NavItem(item: NavItemProps & { href: string }) {
const iconRef = useRef<IconRefType>(null);
const { data: stats } = useStats();
const { data: stats } = useAdjustedStats();

const { state, setOpenMobile } = useSidebar();

Expand Down
11 changes: 4 additions & 7 deletions apps/mail/components/ui/sidebar-labels.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,16 @@ import type { IConnection, Label as LabelType } from '@/types';
import { RecursiveFolder } from './recursive-folder';
import { Tree } from '../magicui/file-tree';
import { useCallback } from 'react';
import { useAdjustedStats } from '@/hooks/use-stats';

type Props = {
data: LabelType[];
activeAccount: IConnection | null | undefined;
stats:
| {
count?: number;
label?: string;
}[]
| undefined;
};

const SidebarLabels = ({ data, activeAccount, stats }: Props) => {
const SidebarLabels = ({ data, activeAccount }: Props) => {
const { data: stats } = useAdjustedStats();

const getLabelCount = useCallback(
(labelName: string | undefined): number => {
if (!stats || !labelName) return 0;
Expand Down
103 changes: 90 additions & 13 deletions apps/mail/hooks/use-mail-navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useOptimisticActions } from './use-optimistic-actions';
import { useMail } from '@/components/mail/use-mail';
import { useHotkeys } from 'react-hotkeys-hook';
import { atom, useAtom } from 'jotai';
import { useQueryState } from 'nuqs';

export const focusedIndexAtom = atom<number | null>(null);
export const mailNavigationCommandAtom = atom<null | 'next' | 'previous'>(null);
Expand All @@ -23,7 +24,10 @@ export function useMailNavigation({ items, containerRef, onNavigate }: UseMailNa
itemsRef.current = items;
const onNavigateRef = useRef(onNavigate);
onNavigateRef.current = onNavigate;
const { open: isCommandPaletteOpen } = useCommandPalette();
const [isCommandPaletteOpen] = useQueryState('isCommandPaletteOpen');

// Track the previously focused thread ID to detect when it gets deleted
const prevFocusedThreadId = useRef<string | null>(null);

const hoveredMailRef = useRef<string | null>(null);
const keyboardActiveRef = useRef(false);
Expand All @@ -39,6 +43,7 @@ export function useMailNavigation({ items, containerRef, onNavigate }: UseMailNa
setFocusedIndex(null);
onNavigateRef.current(null);
keyboardActiveRef.current = false;
prevFocusedThreadId.current = null;
}, [setFocusedIndex, onNavigateRef]);

const getThreadElement = useCallback(
Expand Down Expand Up @@ -77,6 +82,9 @@ export function useMailNavigation({ items, containerRef, onNavigate }: UseMailNa
const message = itemsRef.current[index];
const threadId = message.id;

// Update the tracked focused thread ID
prevFocusedThreadId.current = threadId;

const currentThreadId = window.location.search.includes('threadId=');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: This check may return true for any query param containing 'threadId='. Use URLSearchParams to properly parse the query string

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

if (currentThreadId) {
onNavigateRef.current(threadId);
Expand All @@ -98,36 +106,101 @@ export function useMailNavigation({ items, containerRef, onNavigate }: UseMailNa
const firstItem = itemsRef.current[0];
if (firstItem) {
onNavigateRef.current(firstItem.id);
prevFocusedThreadId.current = firstItem.id;
}
scrollIntoView(0, 'auto');
return 0;
}
onNavigateRef.current(null);
prevFocusedThreadId.current = null;
return null;
}

if (prevIndex < itemsRef.current.length - 1) {
const newIndex = prevIndex;
const nextItem = itemsRef.current[prevIndex + 1];
// Current focused index is beyond the available items (thread was deleted)
if (prevIndex >= itemsRef.current.length) {
const newIndex = Math.max(0, itemsRef.current.length - 1);
const nextItem = itemsRef.current[newIndex];
if (nextItem) {
onNavigateRef.current(nextItem.id);
prevFocusedThreadId.current = nextItem.id;
scrollIntoView(newIndex, 'auto');
return newIndex;
} else {
onNavigateRef.current(null);
prevFocusedThreadId.current = null;
return null;
}
scrollIntoView(newIndex, 'auto');
return newIndex;
} else {
const newIndex = itemsRef.current.length > 1 ? prevIndex - 1 : null;
}

if (newIndex !== null) {
// Check if the focused thread was deleted by comparing thread IDs
const currentItem = itemsRef.current[prevIndex];
const currentThreadId = currentItem?.id;
const wasFocusedThreadDeleted = prevFocusedThreadId.current &&
(!currentThreadId || currentThreadId !== prevFocusedThreadId.current);

if (wasFocusedThreadDeleted) {
// The focused thread was deleted, navigate to the same position
// which now contains the next email in the list
if (prevIndex < itemsRef.current.length) {
const nextItem = itemsRef.current[prevIndex];
if (nextItem) {
onNavigateRef.current(nextItem.id);
prevFocusedThreadId.current = nextItem.id;
scrollIntoView(prevIndex, 'auto');
return prevIndex;
}
}

// If no item at current position, try the previous position
if (prevIndex > 0) {
const newIndex = prevIndex - 1;
const nextItem = itemsRef.current[newIndex];
if (nextItem) {
onNavigateRef.current(nextItem.id);
prevFocusedThreadId.current = nextItem.id;
scrollIntoView(newIndex, 'auto');
return newIndex;
}
}

onNavigateRef.current(null);
prevFocusedThreadId.current = null;
return null;
} else if (currentItem) {
// Current item existts and wasnt deleted, then navigate to the next one
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a typo in this comment: existts should be exists and wasnt should be wasn't. The corrected comment would be: "Current item exists and wasn't deleted, then navigate to the next one"

Suggested change
// Current item existts and wasnt deleted, then navigate to the next one
// Current item exists and wasn't deleted, then navigate to the next one

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BRO STOP

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix typo: "existts" should be "exists"

-        // Current item existts and wasnt deleted, then navigate to the next one
+        // Current item exists and wasn't deleted, then navigate to the next one
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Current item existts and wasnt deleted, then navigate to the next one
// Current item exists and wasn't deleted, then navigate to the next one
🤖 Prompt for AI Agents
In apps/mail/hooks/use-mail-navigation.ts at line 170, fix the typo in the
comment by changing "existts" to "exists" to correct the spelling.

if (prevIndex < itemsRef.current.length - 1) {
const newIndex = prevIndex + 1;
const nextItem = itemsRef.current[newIndex];
if (nextItem) {
onNavigateRef.current(nextItem.id);
prevFocusedThreadId.current = nextItem.id;
}
scrollIntoView(newIndex, 'auto');
return newIndex;
} else {
onNavigateRef.current(null);
return null;
// we're at the end so stay at the current thread
onNavigateRef.current(currentItem.id);
prevFocusedThreadId.current = currentItem.id;
scrollIntoView(prevIndex, 'auto');
return prevIndex;
}
} else {
// no current item so try to find any available thread
if (itemsRef.current.length > 0) {
const newIndex = Math.min(prevIndex, itemsRef.current.length - 1);
const nextItem = itemsRef.current[newIndex];
if (nextItem) {
onNavigateRef.current(nextItem.id);
prevFocusedThreadId.current = nextItem.id;
scrollIntoView(newIndex, 'auto');
return newIndex;
}
}

// 0 threads available
onNavigateRef.current(null);
prevFocusedThreadId.current = null;
return null;
}
});
}, [onNavigateRef, scrollIntoView, setFocusedIndex]);
Expand Down Expand Up @@ -186,18 +259,22 @@ export function useMailNavigation({ items, containerRef, onNavigate }: UseMailNa
if (focusedIndex === null) return;

const message = itemsRef.current[focusedIndex];
if (message) onNavigateRef.current(message.id);
if (message) {
onNavigateRef.current(message.id);
prevFocusedThreadId.current = message.id;
}
}, [focusedIndex]);

const handleEscape = useCallback(() => {
setFocusedIndex(null);
onNavigateRef.current(null);
keyboardActiveRef.current = false;
prevFocusedThreadId.current = null;
}, [setFocusedIndex, onNavigateRef]);

useHotkeys('ArrowUp', handleArrowUp, { preventDefault: true, enabled: !isCommandPaletteOpen });
useHotkeys('ArrowDown', handleArrowDown, { preventDefault: true, enabled: !isCommandPaletteOpen });
useHotkeys('j', handleArrowDown,{enabled: !isCommandPaletteOpen });
useHotkeys('j', handleArrowDown, { enabled: !isCommandPaletteOpen });
useHotkeys('k', handleArrowUp, { enabled: !isCommandPaletteOpen });
useHotkeys('Enter', handleEnter, { preventDefault: true,enabled: !isCommandPaletteOpen });
useHotkeys('Escape', handleEscape, { preventDefault: true,enabled: !isCommandPaletteOpen });
Expand Down
2 changes: 2 additions & 0 deletions apps/mail/hooks/use-optimistic-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ export function useOptimisticActions() {
const optimisticId = addOptimisticAction({
type: 'MOVE',
threadIds,
source: currentFolder,
destination,
});

Expand Down Expand Up @@ -335,6 +336,7 @@ export function useOptimisticActions() {
const optimisticId = addOptimisticAction({
type: 'MOVE',
threadIds,
source: currentFolder,
destination: 'bin',
});

Expand Down
49 changes: 49 additions & 0 deletions apps/mail/hooks/use-stats.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { useTRPC } from '@/providers/query-provider';
import { useQuery } from '@tanstack/react-query';
import { useSession } from '@/lib/auth-client';
import { useAtomValue } from 'jotai';
import { optimisticActionsAtom } from '@/store/optimistic-updates';
import { useMemo } from 'react';

export const useStats = () => {
const { data: session } = useSession();
Expand All @@ -15,3 +18,49 @@ export const useStats = () => {

return statsQuery;
};

export const useAdjustedStats = () => {
const { data: session } = useSession();
const trpc = useTRPC();
const optimisticActions = useAtomValue(optimisticActionsAtom);

const statsQuery = useQuery(
trpc.mail.count.queryOptions(void 0, {
enabled: !!session?.user.id,
staleTime: 1000 * 60 * 60,
}),
);

const adjustedStats = useMemo(() => {
if (!statsQuery.data) return statsQuery.data;

const deltas: Record<string, number> = {};

Object.values(optimisticActions).forEach((action) => {
if (action.type !== 'MOVE') return;

const source = action.source.toLowerCase();
const destination = action.destination?.toLowerCase();
Comment on lines +42 to +43
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: source is accessed without null check which could crash if action.source is undefined

Suggested change
const source = action.source.toLowerCase();
const destination = action.destination?.toLowerCase();
const source = action.source?.toLowerCase();
const destination = action.destination?.toLowerCase();


if (source) {
deltas[source] = (deltas[source] ?? 0) - action.threadIds.length;
}
if (destination && destination !== source) {
deltas[destination] = (deltas[destination] ?? 0) + action.threadIds.length;
}
});

return statsQuery.data.map((stat) => {
const delta = deltas[stat.label?.toLowerCase() ?? ''] ?? 0;
return {
...stat,
count: Math.max(0, (stat.count ?? 0) + delta),
};
});
}, [statsQuery.data, optimisticActions]);

return {
...statsQuery,
data: adjustedStats,
};
};
Loading