-
Notifications
You must be signed in to change notification settings - Fork 1.5k
ui: update the chrome extension visibilty #448
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.
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
ui: update the chrome extension visibilty #448
Conversation
How to use the Graphite Merge QueueAdd the label Main to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| 🔵 In progress View logs |
supermemory-app | 353a6fb | Oct 03 2025, 12:34 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
supermemory-app | 050b241 | Oct 03 2025, 08:02 AM |
Merge activity
|
ui: update the chrome extension visibilty fix the ui for the chrome extension icon remove bun lock  
353a6fb to
050b241
Compare
PR Review: Chrome Extension Visibility UpdateThanks for working on improving the Chrome extension visibility! Here's my review: ✅ Positive Aspects
🐛 Potential Issues1. Import Inconsistency (Critical)Location: import { motion } from "framer-motion"But import { motion } from "motion/react"The package.json shows both Recommendation: Use 2. Race Condition in Extension DetectionLocation: The current implementation has potential timing issues:
Recommendation: useEffect(() => {
let mounted = true;
const checkExtension = () => {
// ... existing code ...
const handleMessage = (event: MessageEvent) => {
if (mounted && event.data?.action === "extension-detected") {
clearTimeout(timeout)
setIsExtensionInstalled(true)
setIsChecking(false)
window.removeEventListener("message", handleMessage)
}
}
// ... rest of code
}
checkExtension()
return () => {
mounted = false
// ... existing cleanup
}
}, [])3. Auto-Minimize Logic IssueLocation: The auto-minimize logic is nested inside the timeout callback, which means it only triggers when the extension is NOT installed. However, the logic seems correct for the use case. Just worth noting that if a user dismisses quickly, the minimize timer still runs. Recommendation: Clear the minimize timeout on dismiss: const [minimizeTimeout, setMinimizeTimeout] = useState<NodeJS.Timeout | null>(null)
// In timeout callback
const timeout = setTimeout(() => {
// ... existing code ...
if (!dismissed) {
const minTimeout = setTimeout(() => setIsMinimized(true), 3000)
setMinimizeTimeout(minTimeout)
}
}, 1000)
// In handleDismiss
const handleDismiss = () => {
if (minimizeTimeout) clearTimeout(minimizeTimeout)
localStorage.setItem("chrome-extension-dismissed", "true")
setIsDismissed(true)
}4. Missing Image OptimizationLocation: Using Concerns:
Recommendation: import extensionLogo from "@/public/images/extension-logo.png"
import extensionBg from "@/public/images/extension-bg.png"
// Then use:
<Image
src={extensionLogo}
alt="Extension Logo"
width={24}
height={24}
/>
// For background:
style={{
backgroundImage: `url(${extensionBg.src})`,
// ... rest
}}5. Accessibility IssuesLocation: Multiple locations in component
Recommendation: <Button
variant="ghost"
size="sm"
onClick={handleDismiss}
aria-label="Dismiss extension prompt"
className="..."
>
<CircleX className="w-4 h-4" />
</Button>
<motion.div
role="dialog"
aria-label="Chrome extension installation prompt"
className="fixed bottom-4 right-4 z-50"
// ...
>6. Header Icons Without ContextLocation: Adding icons to dropdown menu items is great, but importing several icons that aren't used elsewhere in the file: import { User, CreditCard, Chrome, LogOut } from "lucide-react"These are only used for the dropdown menu. Consider if this adds unnecessary bundle size. However, with tree-shaking this is likely fine. 🎨 Style & Best Practices
const EXTENSION_URL = "https://chromewebstore.google.com/detail/supermemory/afpgkkipfdpeaflnpoaffkcankadgjfc"
interface ExtensionMessage {
action: 'check-extension' | 'extension-detected'
}🧪 Testing ConcernsMissing test coverage for:
🔒 SecurityLow risk, but worth noting:
⚡ Performance
const [isDismissed, setIsDismissed] = useState(() =>
typeof window !== 'undefined'
? localStorage.getItem("chrome-extension-dismissed") === "true"
: false
)📝 SummaryOverall: Good implementation with nice UX touches! The main issues are:
The functionality looks solid, but addressing the race condition and accessibility issues would make this production-ready. Recommendation: Request changes for items #1, #2, #4, and #5 before merging. |


ui: update the chrome extension visibilty
fix the ui for the chrome extension icon
remove bun lock