Make automation page the primary assistant page again#468
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update refactors navigation across the web app, changing most references and links from "/assistant" to "/automation". It introduces a side panel toggle in the assistant chat, managed via a "tab" URL query parameter, and adds new UI components for opening and closing this panel. Unused example-related code is removed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatUI
participant URL (Query State)
participant AssistantTabs
User->>ChatUI: Clicks "OpenArtifactButton"
ChatUI->>URL (Query State): Set tab="rules"
ChatUI->>AssistantTabs: Rendered with tab="rules" (side panel opens)
User->>AssistantTabs: Clicks "CloseArtifactButton"
AssistantTabs->>URL (Query State): Set tab=null
ChatUI->>AssistantTabs: Side panel closes (tab cleared)
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/components/assistant-chat/chat.tsxOops! Something went wrong! :( ESLint: 9.24.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/completed/page.tsx (1)
39-41: Update button text to match navigation target.The button text says "Go to Assistant" but now navigates to the automation page. This inconsistency could confuse users about where they're being taken.
Consider updating the button text to match the new navigation target:
- Go to Assistant + Go to AutomationAlternatively, if users should still think of this as going to the "Assistant" experience that's now housed under automation, you might want to keep the text but add clarity through the onboarding messaging.
apps/web/components/assistant-chat/chat.tsx (2)
151-155: Consider adjusting panel size allocation.The chat panel gets 40% width while the side panel gets 60%. This seems backwards - typically the main content (chat) should get more space than the auxiliary panel. Consider swapping these values or making them configurable.
- <ResizablePanel className="overflow-y-auto" defaultSize={40}> + <ResizablePanel className="overflow-y-auto" defaultSize={60}> {chatPanel} </ResizablePanel> <ResizableHandle withHandle /> - <ResizablePanel defaultSize={60}> + <ResizablePanel defaultSize={40}>
306-306: Consider making the tab value configurable.The
OpenArtifactButtonhardcodes"rules"as the tab value. Consider making this configurable or using a constant to improve maintainability and reduce coupling.+ const DEFAULT_TAB = "rules"; + const handleOpenArtifact = () => setTab("rules");Or make it configurable:
- function OpenArtifactButton() { + function OpenArtifactButton({ defaultTab = "rules" }: { defaultTab?: string }) { const [tab, setTab] = useQueryState("tab"); if (tab) return null; - const handleOpenArtifact = () => setTab("rules"); + const handleOpenArtifact = () => setTab(defaultTab);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/web/app/(app)/[emailAccountId]/assistant/AssistantTabs.tsx(4 hunks)apps/web/app/(app)/[emailAccountId]/assistant/examples.ts(0 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/completed/page.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/automation/page.tsx(3 hunks)apps/web/app/(app)/[emailAccountId]/setup/page.tsx(1 hunks)apps/web/components/NavBottom.tsx(1 hunks)apps/web/components/SideNav.tsx(2 hunks)apps/web/components/assistant-chat/chat.tsx(10 hunks)apps/web/components/assistant-chat/overview.tsx(1 hunks)apps/web/components/email-list/EmailList.tsx(1 hunks)apps/web/next.config.ts(1 hunks)apps/web/utils/actions/rule.ts(4 hunks)
💤 Files with no reviewable changes (1)
- apps/web/app/(app)/[emailAccountId]/assistant/examples.ts
🧰 Additional context used
🧬 Code Graph Analysis (6)
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/completed/page.tsx (1)
apps/web/utils/path.ts (1)
prefixPath(1-4)
apps/web/components/email-list/EmailList.tsx (1)
apps/web/utils/path.ts (1)
prefixPath(1-4)
apps/web/utils/actions/rule.ts (1)
apps/web/utils/path.ts (1)
prefixPath(1-4)
apps/web/app/(app)/[emailAccountId]/automation/page.tsx (1)
apps/web/utils/path.ts (1)
prefixPath(1-4)
apps/web/components/SideNav.tsx (1)
apps/web/utils/path.ts (1)
prefixPath(1-4)
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx (1)
apps/web/utils/path.ts (1)
prefixPath(1-4)
🔇 Additional comments (10)
apps/web/components/NavBottom.tsx (1)
52-52: LGTM! Navigation path updated consistently.The path change from "/assistant" to "/automation" aligns with the PR objective. Good decision to keep the user-facing label as "Assistant" while updating the underlying route - this maintains UI consistency and user understanding.
apps/web/app/(app)/[emailAccountId]/setup/page.tsx (1)
107-107: LGTM! Feature card link updated consistently.The href change from "/assistant" to "/automation" is consistent with the PR's routing refactor. The user-facing title and description appropriately maintain the "AI Assistant" terminology while updating the underlying route.
apps/web/components/email-list/EmailList.tsx (1)
130-130: LGTM! Alert link updated to point to automation page.The href change from "/assistant" to "/automation" is consistent with the PR's routing refactor. The link text appropriately keeps the "Assistant page" terminology for user clarity while updating the underlying route. The use of
prefixPathutility function is correct for constructing the emailAccountId-prefixed path.apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx (1)
150-150: LGTM! Navigation change is consistent with the PR objective.The Skip button now correctly navigates to the automation page, which aligns with making automation the primary assistant page.
apps/web/utils/actions/rule.ts (1)
251-251: LGTM! Cache revalidation for automation path is well-implemented.Adding
revalidatePathfor the automation route ensures that rule changes are properly reflected in the automation interface. The implementation is consistent across all rule action functions and maintains the existing assistant path revalidation.Also applies to: 292-292, 313-313, 346-346
apps/web/components/SideNav.tsx (1)
76-76: LGTM! Navigation updates are consistent with the PR objective.Both the main "Assistant" navigation item and the mail navigation "Back" button now correctly point to the automation page, successfully making automation the primary assistant experience.
Also applies to: 251-251
apps/web/app/(app)/[emailAccountId]/assistant/AssistantTabs.tsx (1)
109-119: LGTM! Well-implemented close button component.The CloseArtifactButton is correctly implemented with proper use of
useQueryStatefor managing the tab query parameter anduseCallbackfor memoizing the onClose handler. The component integrates well with the overall side panel toggle functionality.apps/web/app/(app)/[emailAccountId]/automation/page.tsx (1)
103-108: LGTM! Clean implementation of the AI Chat button.The button implementation is correct, using proper imports, appropriate styling with
primaryBluevariant, and theprefixPathutility for href construction. The MessageCircleIcon provides good visual context for the chat functionality.apps/web/components/assistant-chat/overview.tsx (1)
35-39: LGTM! Good UI simplification.The removal of example buttons while keeping the
ExamplesDialogcreates a cleaner, more focused interface. This simplification aligns well with the overall refactoring effort to streamline the assistant chat experience.apps/web/components/assistant-chat/chat.tsx (1)
74-77: Excellent implementation of the side panel toggle system.The integration of
useQueryStatesto manage both input and tab parameters, along with the conditional rendering of theResizablePanelGroup, creates a clean and intuitive side panel toggle mechanism. The coordination betweenOpenArtifactButtonandCloseArtifactButton(in AssistantTabs.tsx) provides a smooth user experience.Also applies to: 146-164, 301-316
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/components/assistant-chat/chat.tsx (1)
184-188: Verify the placement of the error message.The error message for when chat is too long appears to be positioned above the action buttons, which may not be the most user-friendly placement. Consider moving it to a more prominent location or styling it as a banner.
<div className="flex h-full min-w-0 flex-col bg-background"> - <div className="flex items-center justify-between px-2 pt-2"> - {messages.length > MAX_MESSAGES && ( - <div className="rounded-md border border-red-200 bg-red-100 p-2 text-sm text-red-800"> - The chat is too long. Please start a new conversation. - </div> - )} + {messages.length > MAX_MESSAGES && ( + <div className="mx-2 mt-2 rounded-md border border-red-200 bg-red-100 p-2 text-sm text-red-800"> + The chat is too long. Please start a new conversation. + </div> + )} + <div className="flex items-center justify-between px-2 pt-2">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/components/assistant-chat/chat.tsx(7 hunks)
🔇 Additional comments (5)
apps/web/components/assistant-chat/chat.tsx (5)
74-77: Good refactor to useQueryStates for better state management.The change from
useSearchParamstouseQueryStateswith proper parsing for bothinputandtabparameters improves type safety and state management consistency.
146-164: Well-implemented conditional rendering for the side panel.The logic correctly shows either just the chat panel or a resizable panel group with both chat and side panel based on the
tabstate. The default sizing (40% chat, 60% side panel) and mobile-responsive direction handling are appropriate.
243-258: Well-implemented OpenArtifactButton with proper conditional visibility.The component correctly hides when the side panel is already open and sets the tab to "rules" when clicked. The tooltip and accessibility attributes are properly implemented.
190-196: Good consolidation of chat controls.The simplified control bar with
NewChatButton,ExamplesDialog,ChatHistoryDropdown, andOpenArtifactButtonprovides a clean interface. The proper wrapping ofChatHistoryDropdowninSWRProvidermaintains the SWR workaround strategy mentioned in the comments.
206-206:Details
❓ Verification inconclusive
Verify hardcoded isArtifactVisible prop.
The
isArtifactVisibleprop is hardcoded tofalse. Should this be dynamically determined based on thetabstate instead?
🏁 Script executed:
#!/bin/bash # Check if isArtifactVisible should be based on tab state rg -A 5 -B 5 "isArtifactVisible" --type tsx --type tsLength of output: 81
I couldn’t locate any matches using
--type tsx. Let’s try a broad search across all files to pinpoint the usage:
🏁 Script executed:
#!/bin/bash # Search for all occurrences of isArtifactVisible without file‐type filters rg isArtifactVisible -A 5 -B 5Length of output: 2623
Let’s inspect the surrounding code and see how (or if) the tab state is defined:
🏁 Script executed:
#!/bin/bash # Show lines around the hardcoded prop echo "=== context around line 206 ===" sed -n '180,240p' apps/web/components/assistant-chat/chat.tsx # Find all useState calls to locate any `tab` state echo "=== useState declarations in chat.tsx ===" rg -n "useState" apps/web/components/assistant-chat/chat.tsxLength of output: 2169
Confirm artifact visibility behavior
I don’t see any
tabor visibility state inchat.tsx, soisArtifactVisibleis alwaysfalse. Please clarify whether artifacts should remain hidden or be toggled dynamically (e.g. via a new state or context). If you intend to show/hide artifacts, you could:
- In
apps/web/components/assistant-chat/chat.tsx(around line 206), add:const [isArtifactVisible, setIsArtifactVisible] = useState(false);- Pass the state instead of
false:- isArtifactVisible={false} + isArtifactVisible={isArtifactVisible}- Wire up
OpenArtifactButton(or another control) to callsetIsArtifactVisible.Otherwise, if this is intentionally always
false, please add a comment or remove the prop until dynamic support is added.
Summary by CodeRabbit
New Features
Improvements
User Interface