Skip to content

fix: Makes RooTips appear in home again #7986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

fix: Makes RooTips appear in home again #7986

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6af9324
Makes upsell dialog visibility available outside the component to fix…
brunobergher Sep 15, 2025
f10bbef
fix: address PR review feedback
roomote Sep 15, 2025
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
12 changes: 8 additions & 4 deletions webview-ui/src/components/chat/ChatView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import RooTips from "@src/components/welcome/RooTips"
import { StandardTooltip } from "@src/components/ui"
import { useAutoApprovalState } from "@src/hooks/useAutoApprovalState"
import { useAutoApprovalToggles } from "@src/hooks/useAutoApprovalToggles"
import { CloudUpsellDialog } from "@src/components/cloud/CloudUpsellDialog"

import TelemetryBanner from "../common/TelemetryBanner"
import VersionIndicator from "../common/VersionIndicator"
Expand All @@ -55,9 +54,12 @@ import SystemPromptWarning from "./SystemPromptWarning"
import ProfileViolationWarning from "./ProfileViolationWarning"
import { CheckpointWarning } from "./CheckpointWarning"
import { QueuedMessages } from "./QueuedMessages"
import { Cloud } from "lucide-react"

import { CloudUpsellDialog } from "@src/components/cloud/CloudUpsellDialog"
import DismissibleUpsell from "../common/DismissibleUpsell"
import { useCloudUpsell } from "@src/hooks/useCloudUpsell"
import { Cloud } from "lucide-react"
import { useUpsellVisibility, UPSELL_IDS } from "@/hooks/useUpsellVisibility"
Copy link
Contributor

@roomote roomote bot Sep 15, 2025

Choose a reason for hiding this comment

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

Minor: Import path inconsistency. Some imports use @/ alias while others use @src/. Consider standardizing on one approach for consistency.


export interface ChatViewProps {
isHidden: boolean
Expand Down Expand Up @@ -1772,6 +1774,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro

const areButtonsVisible = showScrollToBottom || primaryButtonText || secondaryButtonText || isStreaming

const isCloudUpsellVisible = useUpsellVisibility(UPSELL_IDS.TASK_LIST)

return (
<div
data-testid="chat-view"
Expand Down Expand Up @@ -1842,12 +1846,12 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
<RooHero />

<div className="mb-2.5">
{cloudIsAuthenticated || taskHistory.length < 4 ? (
{cloudIsAuthenticated || (tasks.length === 0 && !isCloudUpsellVisible) ? (
<RooTips />
) : (
<>
<DismissibleUpsell
upsellId="taskList"
upsellId={UPSELL_IDS.TASK_LIST}
icon={<Cloud className="size-4 mt-0.5 shrink-0" />}
onClick={() => openUpsell()}
dismissOnClick={false}
Expand Down
6 changes: 4 additions & 2 deletions webview-ui/src/components/chat/TaskHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { memo, useEffect, useRef, useState } from "react"
import { useTranslation } from "react-i18next"
import { useCloudUpsell } from "@src/hooks/useCloudUpsell"
import { CloudUpsellDialog } from "@src/components/cloud/CloudUpsellDialog"
import DismissibleUpsell from "@src/components/common/DismissibleUpsell"
import { FoldVertical, ChevronUp, ChevronDown } from "lucide-react"
import prettyBytes from "pretty-bytes"

Expand All @@ -24,6 +23,9 @@ import { ContextWindowProgress } from "./ContextWindowProgress"
import { Mention } from "./Mention"
import { TodoListDisplay } from "./TodoListDisplay"

import DismissibleUpsell from "@src/components/common/DismissibleUpsell"
import { UPSELL_IDS } from "@/constants/upsellIds"

export interface TaskHeaderProps {
task: ClineMessage
tokensIn: number
Expand Down Expand Up @@ -103,7 +105,7 @@ const TaskHeader = ({
<div className="pt-2 pb-0 px-3">
{showLongRunningTaskMessage && !isTaskComplete && (
<DismissibleUpsell
upsellId="longRunningTask"
upsellId={UPSELL_IDS.LONG_RUNNING_TASK}
onClick={() => openUpsell()}
dismissOnClick={false}
variant="banner">
Expand Down
69 changes: 24 additions & 45 deletions webview-ui/src/components/common/DismissibleUpsell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { memo, ReactNode, useEffect, useState, useRef } from "react"
import { vscode } from "@src/utils/vscode"
import { memo, ReactNode } from "react"
import { useAppTranslation } from "@src/i18n/TranslationContext"
import { DismissedUpsellsProvider, useDismissedUpsells } from "@src/context/DismissedUpsellsContext"

interface DismissibleUpsellProps {
/** Required unique identifier for this upsell */
Expand Down Expand Up @@ -32,7 +32,8 @@ const DismissIcon = () => (
</svg>
)

const DismissibleUpsell = memo(
// Internal component that uses the context
const DismissibleUpsellInternal = memo(
({
upsellId,
className,
Expand All @@ -44,55 +45,21 @@ const DismissibleUpsell = memo(
dismissOnClick = false,
}: DismissibleUpsellProps) => {
const { t } = useAppTranslation()
const [isVisible, setIsVisible] = useState(false)
const isMountedRef = useRef(true)
const { isUpsellVisible, dismissUpsell, isLoading } = useDismissedUpsells()

useEffect(() => {
// Track mounted state
isMountedRef.current = true
// Check if this upsell is visible
const isVisible = isUpsellVisible(upsellId)

// Request the current list of dismissed upsells from the extension
vscode.postMessage({ type: "getDismissedUpsells" })

// Listen for the response
const handleMessage = (event: MessageEvent) => {
// Only update state if component is still mounted
if (!isMountedRef.current) return

const message = event.data
// Add null/undefined check for message
if (message && message.type === "dismissedUpsells" && Array.isArray(message.list)) {
// Check if this upsell has been dismissed
if (!message.list.includes(upsellId)) {
setIsVisible(true)
}
}
}

window.addEventListener("message", handleMessage)
return () => {
isMountedRef.current = false
window.removeEventListener("message", handleMessage)
}
}, [upsellId])

const handleDismiss = async () => {
// First notify the extension to persist the dismissal
// This ensures the message is sent even if the component unmounts quickly
vscode.postMessage({
type: "dismissUpsell",
upsellId: upsellId,
})

// Then hide the upsell
setIsVisible(false)
const handleDismiss = () => {
// Dismiss the upsell through the context
dismissUpsell(upsellId)

// Call the optional callback
onDismiss?.()
}

// Don't render if not visible
if (!isVisible) {
// Don't render if not visible or still loading
if (!isVisible || isLoading) {
return null
}

Expand All @@ -107,6 +74,7 @@ const DismissibleUpsell = memo(
button: "text-vscode-notifications-foreground",
},
}

// Build container classes based on variant and presence of click handler
const containerClasses = [
"relative flex items-start justify-between gap-2",
Expand Down Expand Up @@ -158,6 +126,17 @@ const DismissibleUpsell = memo(
},
)

DismissibleUpsellInternal.displayName = "DismissibleUpsellInternal"

// Wrapper component that provides the context
const DismissibleUpsell = memo((props: DismissibleUpsellProps) => {
return (
<DismissedUpsellsProvider>
Copy link
Contributor

@roomote roomote bot Sep 15, 2025

Choose a reason for hiding this comment

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

Could we consider lifting the DismissedUpsellsProvider to a higher level? Each DismissibleUpsell instance wrapping itself with a provider could cause issues if multiple upsells are rendered, as each would have its own provider instance and separate state.

The provider should ideally be at the app root level to share state across all upsells.

Copy link
Collaborator Author

@brunobergher brunobergher Sep 15, 2025

Choose a reason for hiding this comment

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

I did, but this isn't used often enough to warrant being at the top level. If other reviewers agree with you, I can make that change, but I'll refrain from it for now.

<DismissibleUpsellInternal {...props} />
</DismissedUpsellsProvider>
)
})

DismissibleUpsell.displayName = "DismissibleUpsell"

export default DismissibleUpsell
Loading
Loading