ADE Code TUI: kill drawer flicker + marketing site refresh#594
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 15 minutes and 38 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (68)
📒 Files selected for processing (65)
📝 WalkthroughWalkthroughTwo independent workstreams: the TUI CLI drawer is simplified (single-row ChangesTUI CLI: Drawer layout, app helpers, and rendering
Web: OgImage route, FeatureGrid video catalog, ShipShowcase, asset migration
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
arul28
changed the title
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
June 16, 2026 22:57
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/editorial/ShipShowcase.tsx (1)
706-719:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIncomplete ARIA tab pattern is misleading for keyboard/screen-reader users.
Line 706 and Line 716 declare tab semantics, but the required tab-panel linkage/keyboard behavior is missing. Either fully implement the tabs pattern or use plain button-group semantics.
Suggested minimal fix (use button-group semantics instead of partial tabs)
- <div - role="tablist" - aria-label="Product walkthrough" - className="flex flex-wrap gap-2 sm:gap-3" - > + <div + role="group" + aria-label="Product walkthrough" + className="flex flex-wrap gap-2 sm:gap-3" + > @@ - role="tab" - aria-selected={selected} + aria-pressed={selected} onClick={() => selectTab(index)}As per coding guidelines, "
apps/web/**: Web app — check for accessibility, SSR compatibility, and proper React patterns."Also applies to: 761-773
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/editorial/ShipShowcase.tsx` around lines 706 - 719, The ShipShowcase component declares incomplete ARIA tab semantics with role="tablist" on the parent container and role="tab" on individual buttons, but lacks the required aria-controls linkage to tab-panels and keyboard navigation behavior, which is misleading for accessibility. Replace the incomplete tab pattern with simple button-group semantics by removing the role="tablist" from the parent container and the role="tab" and aria-selected attributes from individual buttons in the map function, keeping the aria-label on the parent for context. This approach applies to both the main tab button group (around line 706-719) and the secondary tab group (around line 761-773).Source: Coding guidelines
apps/ade-cli/src/tuiClient/app.tsx (1)
6927-6971:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid rebinding the PTY stream on
refreshStatechurn.This subscription uses
replay: false, so tearing it down wheneverrefreshStatechanges can drop livepty_dataemitted during the resubscribe gap. Use the existingrefreshStateRefforpty_exitand keep the effect keyed toconnection.Proposed fix
} if (payload.type === "pty_exit") { - void refreshState({ hydrateHistory: false }).catch(() => undefined); + void refreshStateRef.current({ hydrateHistory: false }).catch(() => undefined); } @@ - }, [connection, refreshState]); + }, [connection]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/tuiClient/app.tsx` around lines 6927 - 6971, The effect that subscribes to runtime events with `replay: false` is being torn down and rebound whenever the `refreshState` function changes (due to its presence in the dependency array), which causes live pty_data events to be dropped during the resubscribe gap. Instead of calling `refreshState` directly in the pty_exit handler, use a ref called `refreshStateRef` to access the function, and remove `refreshState` from the effect's dependency array. Keep the effect keyed only to `connection` so the subscription persists across refreshState changes and prevents losing emitted events during resubscription.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ade-cli/src/tuiClient/app.tsx`:
- Around line 532-559: In the noticeScopeId function, reorder the conditional
checks to prioritize draft scope by checking args.draftChatActive and
args.draftScopeKey before checking args.activeSessionId, ensuring draft notices
take precedence. In the selectVisibleNotices function, add validation when
args.draftChatActive is true to reject the case where args.draftScopeKey is
null, preventing global notices from being matched. Ensure that only notices
with the exact draft scope key are returned when in draft chat mode, with no
fallback to global notices.
In `@apps/web/src/components/editorial/FeatureGrid.tsx`:
- Around line 200-257: The Lightbox component uses proper modal semantics with
role="dialog" and aria-modal="true", but lacks focus management required for
accessible modals. Within the useEffect hook in the Lightbox function, capture
the currently focused element before the dialog opens so it can be restored
later. Then move focus into the dialog when it mounts (typically to the close
button). Implement focus trapping to prevent keyboard navigation from leaving
the dialog to background elements while it is open. Finally, restore focus to
the previously focused element in the cleanup function alongside the existing
removeEventListener and overflow restoration code.
In `@apps/web/vercel.json`:
- Around line 3-16: The Cache-Control headers for both the "/videos/(.*)" and
"/images/(.*)" source patterns include the "immutable" directive, which is
unsafe without content-hashed asset filenames as it forces browsers to cache
assets indefinitely. Remove the "immutable" directive from the Cache-Control
values in both patterns while keeping "public, max-age=31536000", allowing
browsers to eventually revalidate or clear stale assets after expiration rather
than serving them indefinitely after deployment updates.
---
Outside diff comments:
In `@apps/ade-cli/src/tuiClient/app.tsx`:
- Around line 6927-6971: The effect that subscribes to runtime events with
`replay: false` is being torn down and rebound whenever the `refreshState`
function changes (due to its presence in the dependency array), which causes
live pty_data events to be dropped during the resubscribe gap. Instead of
calling `refreshState` directly in the pty_exit handler, use a ref called
`refreshStateRef` to access the function, and remove `refreshState` from the
effect's dependency array. Keep the effect keyed only to `connection` so the
subscription persists across refreshState changes and prevents losing emitted
events during resubscription.
In `@apps/web/src/components/editorial/ShipShowcase.tsx`:
- Around line 706-719: The ShipShowcase component declares incomplete ARIA tab
semantics with role="tablist" on the parent container and role="tab" on
individual buttons, but lacks the required aria-controls linkage to tab-panels
and keyboard navigation behavior, which is misleading for accessibility. Replace
the incomplete tab pattern with simple button-group semantics by removing the
role="tablist" from the parent container and the role="tab" and aria-selected
attributes from individual buttons in the map function, keeping the aria-label
on the parent for context. This approach applies to both the main tab button
group (around line 706-719) and the secondary tab group (around line 761-773).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8cbeacdb-574c-49ae-86b1-dd7c9a2131a5
⛔ Files ignored due to path filters (92)
README.mdis excluded by!*.mdapps/web/public/images/ade-dock-icon.pngis excluded by!**/*.pngapps/web/public/images/ade-wordmark.pngis excluded by!**/*.pngapps/web/public/images/competitors/claude-code.pngis excluded by!**/*.pngapps/web/public/images/competitors/codex.pngis excluded by!**/*.pngapps/web/public/images/competitors/conductor.pngis excluded by!**/*.pngapps/web/public/images/competitors/cursor.pngis excluded by!**/*.pngapps/web/public/images/competitors/factory.pngis excluded by!**/*.pngapps/web/public/images/competitors/github.pngis excluded by!**/*.pngapps/web/public/images/competitors/opencode.pngis excluded by!**/*.pngapps/web/public/images/competitors/paperclip.pngis excluded by!**/*.pngapps/web/public/images/competitors/superset.pngis excluded by!**/*.pngapps/web/public/images/competitors/t3-code.pngis excluded by!**/*.pngapps/web/public/images/features/agent-chat.pngis excluded by!**/*.pngapps/web/public/images/features/automations.pngis excluded by!**/*.pngapps/web/public/images/features/cto.pngis excluded by!**/*.pngapps/web/public/images/features/files.pngis excluded by!**/*.pngapps/web/public/images/features/git-history.pngis excluded by!**/*.pngapps/web/public/images/features/lanes.pngis excluded by!**/*.pngapps/web/public/images/features/linear-sync.pngis excluded by!**/*.pngapps/web/public/images/features/modelconfig.pngis excluded by!**/*.pngapps/web/public/images/features/multi-tasking.pngis excluded by!**/*.pngapps/web/public/images/features/prs.pngis excluded by!**/*.pngapps/web/public/images/features/run.pngis excluded by!**/*.pngapps/web/public/images/features/terminals.pngis excluded by!**/*.pngapps/web/public/images/features/workspacegraph.pngis excluded by!**/*.pngapps/web/public/images/hero/hero-desktop.pngis excluded by!**/*.pngapps/web/public/images/hero/hero-mobile.pngis excluded by!**/*.pngapps/web/public/images/hero/hero-tui.pngis excluded by!**/*.pngapps/web/public/images/screenshots/Screenshot 2026-04-02 at 2.10.22 PM.pngis excluded by!**/*.pngapps/web/public/images/screenshots/Screenshot 2026-04-02 at 2.13.22 PM.pngis excluded by!**/*.pngapps/web/public/images/screenshots/agent-chat.pngis excluded by!**/*.pngapps/web/public/images/screenshots/cto.pngis excluded by!**/*.pngapps/web/public/images/screenshots/files copy.pngis excluded by!**/*.pngapps/web/public/images/screenshots/files.pngis excluded by!**/*.pngapps/web/public/images/screenshots/git history.pngis excluded by!**/*.pngapps/web/public/images/screenshots/lanes.pngis excluded by!**/*.pngapps/web/public/images/screenshots/linear-sync.pngis excluded by!**/*.pngapps/web/public/images/screenshots/mobile-pr.pngis excluded by!**/*.pngapps/web/public/images/screenshots/mobile-worktrees.pngis excluded by!**/*.pngapps/web/public/images/screenshots/multi-tasking.pngis excluded by!**/*.pngapps/web/public/images/screenshots/prs.pngis excluded by!**/*.pngapps/web/public/images/screenshots/run.pngis excluded by!**/*.pngapps/web/public/images/screenshots/workspacegraph.pngis excluded by!**/*.pngapps/web/public/images/secondPage/lanesDesktop.pngis excluded by!**/*.pngapps/web/public/images/secondPage/lanesMobile.pngis excluded by!**/*.pngapps/web/public/images/secondPage/lanesTUI.pngis excluded by!**/*.pngapps/web/public/og-image.pngis excluded by!**/*.pngapps/web/public/videos/adecodeDemo.mp4is excluded by!**/*.mp4apps/web/public/videos/autocreateLane.mp4is excluded by!**/*.mp4apps/web/public/videos/browserInspect.mp4is excluded by!**/*.mp4apps/web/public/videos/creatingPRfromChat.mp4is excluded by!**/*.mp4apps/web/public/videos/graphsTab.mp4is excluded by!**/*.mp4apps/web/public/videos/gridViewDemo.mp4is excluded by!**/*.mp4apps/web/public/videos/linearConnection.mp4is excluded by!**/*.mp4apps/web/public/videos/remoteConnect.mp4is excluded by!**/*.mp4apps/web/public/videos/subagentsDemo.mp4is excluded by!**/*.mp4assets/readme/ade-code-tui.gifis excluded by!**/*.gif,!assets/**assets/readme/auto-worktrees.gifis excluded by!**/*.gif,!assets/**assets/readme/cto.webpis excluded by!assets/**assets/readme/grid-view.gifis excluded by!**/*.gif,!assets/**assets/readme/hero-desktop.pngis excluded by!**/*.png,!assets/**assets/readme/hero-iphone.pngis excluded by!**/*.png,!assets/**assets/readme/mobile-chat.webpis excluded by!assets/**assets/readme/mobile-pr.webpis excluded by!assets/**assets/readme/pr-review.webpis excluded by!assets/**assets/readme/worktree-graph.webpis excluded by!assets/**docs/features/ade-code/README.mdis excluded by!docs/**media/demos/ade-code-tui.gifis excluded by!**/*.gifmedia/demos/adecodeDemo.mp4is excluded by!**/*.mp4media/demos/auto-worktrees.gifis excluded by!**/*.gifmedia/demos/autocreateLane.mp4is excluded by!**/*.mp4media/demos/browserInspect.mp4is excluded by!**/*.mp4media/demos/creatingPRfromChat.mp4is excluded by!**/*.mp4media/demos/graphsTab.mp4is excluded by!**/*.mp4media/demos/grid-view.gifis excluded by!**/*.gifmedia/demos/gridViewDemo.mp4is excluded by!**/*.mp4media/demos/linearConnection.mp4is excluded by!**/*.mp4media/demos/remoteConnect.mp4is excluded by!**/*.mp4media/demos/subagentsDemo.mp4is excluded by!**/*.mp4media/logos/models/anthropic.svgis excluded by!**/*.svgmedia/logos/models/claude-color.svgis excluded by!**/*.svgmedia/logos/models/cohere-color.svgis excluded by!**/*.svgmedia/logos/models/deepseek-color.svgis excluded by!**/*.svgmedia/logos/models/gemini-color.svgis excluded by!**/*.svgmedia/logos/models/grok.svgis excluded by!**/*.svgmedia/logos/models/meta-color.svgis excluded by!**/*.svgmedia/logos/models/mistral-color.svgis excluded by!**/*.svgmedia/logos/models/ollama.svgis excluded by!**/*.svgmedia/logos/models/openai.svgis excluded by!**/*.svgmedia/logos/models/perplexity-color.svgis excluded by!**/*.svgmedia/logos/models/qwen-color.svgis excluded by!**/*.svg
📒 Files selected for processing (128)
apps/ade-cli/src/tuiClient/__tests__/Drawer.test.tsxapps/ade-cli/src/tuiClient/__tests__/appInput.test.tsapps/ade-cli/src/tuiClient/__tests__/drawerLayout.test.tsapps/ade-cli/src/tuiClient/app.tsxapps/ade-cli/src/tuiClient/components/Drawer.tsxapps/ade-cli/src/tuiClient/drawerLayout.tsapps/ade-cli/src/tuiClient/theme.tsapps/ade-cli/src/tuiClient/types.tsapps/web/public/images/ade-dock-icon.webpapps/web/public/images/competitors/claude-code.webpapps/web/public/images/competitors/codex.webpapps/web/public/images/competitors/conductor.webpapps/web/public/images/competitors/cursor.webpapps/web/public/images/competitors/factory.webpapps/web/public/images/competitors/github.webpapps/web/public/images/competitors/opencode.webpapps/web/public/images/competitors/paperclip.webpapps/web/public/images/competitors/superset.webpapps/web/public/images/competitors/t3-code.webpapps/web/public/images/hero/hero-desktop.webpapps/web/public/images/hero/hero-mobile.webpapps/web/public/images/hero/hero-tui.webpapps/web/public/images/screenshots/agent-chat.webpapps/web/public/images/screenshots/cto.webpapps/web/public/images/screenshots/lanes.webpapps/web/public/images/screenshots/mobile-pr.webpapps/web/public/images/screenshots/mobile-worktrees.webpapps/web/public/images/screenshots/prs.webpapps/web/public/images/screenshots/run.webpapps/web/public/images/secondPage/chatDesktop.webpapps/web/public/images/secondPage/chatMobile.webpapps/web/public/images/secondPage/chatModelPicker.webpapps/web/public/images/secondPage/chatTui.webpapps/web/public/images/secondPage/lanesDesktop.webpapps/web/public/images/secondPage/lanesMobile.webpapps/web/public/images/secondPage/lanesTUI.webpapps/web/public/images/secondPage/prDesktop.webpapps/web/public/images/secondPage/prMobile.webpapps/web/public/images/secondPage/workBrowser.webpapps/web/public/images/secondPage/workFiles.webpapps/web/public/images/secondPage/workGit.webpapps/web/public/videos/adecodeDemo.webpapps/web/public/videos/autocreateLane.webpapps/web/public/videos/browserInspect.webpapps/web/public/videos/creatingPRfromChat.webpapps/web/public/videos/graphsTab.webpapps/web/public/videos/gridViewDemo.webpapps/web/public/videos/linearConnection.webpapps/web/public/videos/remoteConnect.webpapps/web/public/videos/subagentsDemo.webpapps/web/src/app/SiteRoutes.tsxapps/web/src/app/layout/SiteLayout.tsxapps/web/src/app/pages/HomePage.tsxapps/web/src/components/OgImage.tsxapps/web/src/components/editorial/AnnotatedFigure.tsxapps/web/src/components/editorial/BackCover.tsxapps/web/src/components/editorial/CompetitorEquation.tsxapps/web/src/components/editorial/Cutout.tsxapps/web/src/components/editorial/DeviceComposition.tsxapps/web/src/components/editorial/FeatureGrid.tsxapps/web/src/components/editorial/HeroAgentBadges.tsxapps/web/src/components/editorial/IPhoneFrame.tsxapps/web/src/components/editorial/ShipShowcase.tsxapps/web/src/components/ui/HeroVisual.tsxapps/web/vercel.jsonmedia/README.mdmedia/brand/ade-icon.webpmedia/features/agent-chat/attach-issue.webpmedia/features/agent-chat/chat-or-cli.webpmedia/features/agent-chat/chat-tools-button.webpmedia/features/agent-chat/choose-or-autocreate-lane.webpmedia/features/agent-chat/grid-view.webpmedia/features/agent-chat/launch-in-background.webpmedia/features/agent-chat/model-selection.webpmedia/features/agent-chat/multi-model-prompt.webpmedia/features/agent-chat/new-chat-button.webpmedia/features/agent-chat/run-orchestrator.webpmedia/features/browser/browser-inspect.webpmedia/features/cto/subagents.webpmedia/features/files/choose-worktree.webpmedia/features/files/preview-or-raw.webpmedia/features/git/git-actions.webpmedia/features/linear/linear-connection.webpmedia/features/mobile/agent-chat.webpmedia/features/mobile/pull-requests.webpmedia/features/project/add-project-button.webpmedia/features/project/add-project-options.webpmedia/features/pull-requests/commits-section.webpmedia/features/pull-requests/files-and-ci-checks.webpmedia/features/pull-requests/make-new-pr.webpmedia/features/pull-requests/pr-sources.webpmedia/features/pull-requests/prs-sidebar.webpmedia/features/pull-requests/rebase-options.webpmedia/features/pull-requests/review-tab.webpmedia/features/remote/remote-connect.webpmedia/features/settings/provider-usage.webpmedia/features/settings/settings-tab.webpmedia/features/tui/ade-code.webpmedia/features/worktrees/manage-lane-button.webpmedia/features/worktrees/manage-lane-options.webpmedia/features/worktrees/new-lane-button.webpmedia/features/worktrees/new-lane-options.webpmedia/features/worktrees/worktree-graph.webpmedia/landing/hero/desktop.webpmedia/landing/hero/mobile.webpmedia/landing/hero/tui.webpmedia/landing/showcase/agent-chat-desktop.webpmedia/landing/showcase/agent-chat-mobile.webpmedia/landing/showcase/agent-chat-model-picker.webpmedia/landing/showcase/agent-chat-tui.webpmedia/landing/showcase/pull-requests-desktop.webpmedia/landing/showcase/pull-requests-mobile.webpmedia/landing/showcase/work-tools-browser.webpmedia/landing/showcase/work-tools-files.webpmedia/landing/showcase/work-tools-git.webpmedia/landing/showcase/worktrees-desktop.webpmedia/landing/showcase/worktrees-mobile.webpmedia/landing/showcase/worktrees-tui.webpmedia/logos/competitors/claude-code.webpmedia/logos/competitors/codex.webpmedia/logos/competitors/conductor.webpmedia/logos/competitors/cursor.webpmedia/logos/competitors/factory.webpmedia/logos/competitors/github.webpmedia/logos/competitors/opencode.webpmedia/logos/competitors/paperclip.webpmedia/logos/competitors/superset.webpmedia/logos/competitors/t3-code.webp
Comment on lines
+200
to
+257
| function Lightbox({ demo, onClose }: { demo: Demo; onClose: () => void }) { | ||
| useEffect(() => { | ||
| const onKey = (e: KeyboardEvent) => { | ||
| if (e.key === "Escape") onClose(); | ||
| }; | ||
| window.addEventListener("keydown", onKey); | ||
| const prev = document.body.style.overflow; | ||
| document.body.style.overflow = "hidden"; | ||
| return () => { | ||
| window.removeEventListener("keydown", onKey); | ||
| document.body.style.overflow = prev; | ||
| }; | ||
| }, [onClose]); | ||
|
|
||
| return ( | ||
| <motion.div | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| transition={{ duration: 0.2 }} | ||
| role="dialog" | ||
| aria-modal="true" | ||
| aria-label={`${demo.label} demo`} | ||
| onClick={onClose} | ||
| className="fixed inset-0 z-[100] flex items-center justify-center bg-black/80 p-[clamp(16px,4vw,64px)] backdrop-blur-md" | ||
| > | ||
| <motion.div | ||
| initial={{ scale: 0.96, y: 8 }} | ||
| animate={{ scale: 1, y: 0 }} | ||
| exit={{ scale: 0.97, y: 6 }} | ||
| transition={{ duration: 0.22, ease: [0.22, 1, 0.36, 1] }} | ||
| onClick={(e) => e.stopPropagation()} | ||
| className="relative w-full max-w-[1180px]" | ||
| > | ||
| <button | ||
| type="button" | ||
| onClick={onClose} | ||
| aria-label="Close" | ||
| className="absolute -top-11 right-0 flex items-center gap-1.5 text-[12px] uppercase tracking-[0.16em] text-white/70 transition-colors hover:text-white" | ||
| > | ||
| Close <X className="h-4 w-4" /> | ||
| </button> | ||
| <video | ||
| key={demo.video} | ||
| src={demo.video} | ||
| poster={demo.poster} | ||
| autoPlay | ||
| loop | ||
| controls | ||
| playsInline | ||
| className="block max-h-[78vh] w-full rounded-[4px] border border-white/10 bg-black shadow-[0_40px_120px_-40px_rgba(0,0,0,0.9)]" | ||
| /> | ||
| <div className="mt-4 flex flex-wrap items-baseline gap-x-4 gap-y-1"> | ||
| <h3 className="font-serif text-[22px] text-[color:var(--color-cream)]">{demo.label}</h3> | ||
| <p className="text-[14px] text-[color:var(--color-cream-muted)]">{demo.blurb}</p> | ||
| </div> | ||
| </motion.div> | ||
| </motion.div> |
There was a problem hiding this comment.
Add focus containment and focus restoration for the lightbox dialog.
Line 215 adds modal semantics, but focus is neither moved into the dialog nor trapped there, so keyboard users can tab into background controls while the modal is open.
Suggested fix
function Lightbox({ demo, onClose }: { demo: Demo; onClose: () => void }) {
+ const dialogRef = useRef<HTMLDivElement>(null);
+ const closeRef = useRef<HTMLButtonElement>(null);
+ const previousFocusRef = useRef<HTMLElement | null>(null);
+
useEffect(() => {
+ previousFocusRef.current =
+ document.activeElement instanceof HTMLElement ? document.activeElement : null;
+ closeRef.current?.focus();
+
const onKey = (e: KeyboardEvent) => {
- if (e.key === "Escape") onClose();
+ if (e.key === "Escape") {
+ e.preventDefault();
+ onClose();
+ return;
+ }
+ if (e.key === "Tab") {
+ const root = dialogRef.current;
+ if (!root) return;
+ const focusables = root.querySelectorAll<HTMLElement>(
+ 'button,[href],input,select,textarea,[tabindex]:not([tabindex="-1"])'
+ );
+ if (!focusables.length) return;
+ const first = focusables[0];
+ const last = focusables[focusables.length - 1];
+ if (e.shiftKey && document.activeElement === first) {
+ e.preventDefault();
+ last.focus();
+ } else if (!e.shiftKey && document.activeElement === last) {
+ e.preventDefault();
+ first.focus();
+ }
+ }
};
window.addEventListener("keydown", onKey);
const prev = document.body.style.overflow;
document.body.style.overflow = "hidden";
return () => {
window.removeEventListener("keydown", onKey);
document.body.style.overflow = prev;
+ previousFocusRef.current?.focus();
};
}, [onClose]);
return (
<motion.div
@@
- <motion.div
+ <motion.div
+ ref={dialogRef}
initial={{ scale: 0.96, y: 8 }}
animate={{ scale: 1, y: 0 }}
exit={{ scale: 0.97, y: 6 }}
@@
- <button
+ <button
+ ref={closeRef}
type="button"
onClick={onClose}
aria-label="Close"As per coding guidelines, apps/web/**: Web app — check for accessibility, SSR compatibility, and proper React patterns.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/editorial/FeatureGrid.tsx` around lines 200 - 257,
The Lightbox component uses proper modal semantics with role="dialog" and
aria-modal="true", but lacks focus management required for accessible modals.
Within the useEffect hook in the Lightbox function, capture the currently
focused element before the dialog opens so it can be restored later. Then move
focus into the dialog when it mounts (typically to the close button). Implement
focus trapping to prevent keyboard navigation from leaving the dialog to
background elements while it is open. Finally, restore focus to the previously
focused element in the cleanup function alongside the existing
removeEventListener and overflow restoration code.
Source: Coding guidelines
Comment on lines
+3
to
+16
| "headers": [ | ||
| { | ||
| "source": "/videos/(.*)", | ||
| "headers": [ | ||
| { "key": "Cache-Control", "value": "public, max-age=31536000, immutable" } | ||
| ] | ||
| }, | ||
| { | ||
| "source": "/images/(.*)", | ||
| "headers": [ | ||
| { "key": "Cache-Control", "value": "public, max-age=31536000, immutable" } | ||
| ] | ||
| } | ||
| ], |
There was a problem hiding this comment.
immutable is unsafe here without fingerprinted asset filenames.
Line 7 and Line 13 apply year-long immutable browser caching to /images/* and /videos/*, but the site references stable names (not content-hashed), so media updates can remain stale client-side long after deploy.
Safer cache policy while preserving CDN performance
- { "key": "Cache-Control", "value": "public, max-age=31536000, immutable" }
+ { "key": "Cache-Control", "value": "public, max-age=0, s-maxage=31536000, must-revalidate" }
@@
- { "key": "Cache-Control", "value": "public, max-age=31536000, immutable" }
+ { "key": "Cache-Control", "value": "public, max-age=0, s-maxage=31536000, must-revalidate" }📝 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
| "headers": [ | |
| { | |
| "source": "/videos/(.*)", | |
| "headers": [ | |
| { "key": "Cache-Control", "value": "public, max-age=31536000, immutable" } | |
| ] | |
| }, | |
| { | |
| "source": "/images/(.*)", | |
| "headers": [ | |
| { "key": "Cache-Control", "value": "public, max-age=31536000, immutable" } | |
| ] | |
| } | |
| ], | |
| "headers": [ | |
| { | |
| "source": "/videos/(.*)", | |
| "headers": [ | |
| { "key": "Cache-Control", "value": "public, max-age=0, s-maxage=31536000, must-revalidate" } | |
| ] | |
| }, | |
| { | |
| "source": "/images/(.*)", | |
| "headers": [ | |
| { "key": "Cache-Control", "value": "public, max-age=0, s-maxage=31536000, must-revalidate" } | |
| ] | |
| } | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/vercel.json` around lines 3 - 16, The Cache-Control headers for both
the "/videos/(.*)" and "/images/(.*)" source patterns include the "immutable"
directive, which is unsafe without content-hashed asset filenames as it forces
browsers to cache assets indefinitely. Remove the "immutable" directive from the
Cache-Control values in both patterns while keeping "public, max-age=31536000",
allowing browsers to eventually revalidate or clear stale assets after
expiration rather than serving them indefinitely after deployment updates.
ADE Code (TUI): - Kill drawer flicker: single-line lane cards so selecting a lane only adds a violet border (no row reflow); a shared ChatRow renders the expanded + compact chat lists identically; drop the CHATS header and timestamps; stableInkViewportRows reserves a row to avoid Ink-5 full-screen clears; terminal-preview frame dedup; visible-only PTY buffering with bounded flush. - Sync provider glyphs/colors 1:1 to the model picker; per-draft notice scoping (draft:N); simplified cross-terminal mouse baseline; modified-key word backspace; grid Tab escapes to side panes. Web: - PNG -> WebP image migration; HomePage editorial restructure into FeatureGrid / ShipShowcase; new OgImage /_og card; vercel cache headers for images/videos; brand + demo media assets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
arul28
changed the title
arul28
force-pushed
the
ade/annoyin-flicker-c0ee62d3
branch
from
bb261c7 to
5f67639
Compare
…0ee62d3 # Conflicts: # apps/ade-cli/src/tuiClient/components/Drawer.tsx # apps/web/src/components/editorial/ShipShowcase.tsx
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
- assets/readme/hero.png: force-add the README hero image (gitignored as *.png, so validate-docs failed in CI on the missing target while passing locally). Matches the existing tracked-PNG pattern (og-image.png, logo.png). - vercel.json: drop `immutable` for /images + /videos (stable, non-fingerprinted filenames) in favor of `max-age=0, s-maxage=31536000, must-revalidate` so the CDN still caches a year but browsers pick up media updates. - FeatureGrid Lightbox: add modal focus containment + restoration (focus the close button on open, trap Tab inside the dialog, restore focus on close). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a62d103839
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+6970
to
+6974
| if (!shouldBufferPtyDataForSession({ | ||
| sessionId, | ||
| activeSessionId: activeSessionIdRef.current, | ||
| multiView: multiViewRef.current, | ||
| gridViewActive: gridViewActiveRef.current, |
There was a problem hiding this comment.
When a Claude terminal is kept as a tile in a dormant grid (multiView exists but gridViewActive is false), this new guard drops every pty_data chunk. The resume paths I checked (toggleGridView only calls setGridView(true)) do not re-run hydrateTerminalTilePreview for existing terminal tiles, so hiding the grid while the terminal keeps running and then reopening it shows the stale seed/live buffer and misses all output produced while hidden.
Useful? React with 👍 / 👎.
| const isHome = pathname === "/"; | ||
| // HomePage renders its own editorial Masthead, and /_og is a bare | ||
| // 1200×630 card for screenshotting — both skip the global chrome. | ||
| const isHome = pathname === "/" || pathname === "/_og"; |
There was a problem hiding this comment.
Because the refreshed HomePage no longer renders BackCover/IndexPage, keeping / inside isHome suppresses the only SiteFooter on the production homepage. Non-home routes still get the footer with Privacy/Terms/license links, but / now ends after FeatureGrid; either render the shared footer for / or add a replacement footer to the home page.
Useful? React with 👍 / 👎.
Comment on lines
+4
to
+5
| * Social link-preview card, rendered at exactly 1200×630 and screenshotted to | ||
| * /og-image.png. Mirrors the site hero — the competitor equation, the headline, |
There was a problem hiding this comment.
This component is documented as the source for /og-image.png, but the checked-in generator still defaults to apps/web/scripts/og-image.html, whose image tags reference deleted .png assets such as images/competitors/claude-code.png and images/ade-dock-icon.png. Running scripts/generate-og-image.mjs after this asset conversion will regenerate a social card with broken logos instead of this new /_og route.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+6970
to
+6977
| if (!shouldBufferPtyDataForSession({ | ||
| sessionId, | ||
| activeSessionId: activeSessionIdRef.current, | ||
| multiView: multiViewRef.current, | ||
| gridViewActive: gridViewActiveRef.current, | ||
| })) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This branch drops pty_data events for terminal sessions unless they are currently active or visible in the grid. When a Claude Code terminal keeps running after the user switches to another chat, lane, or hides a saved grid, its live output is discarded instead of retained for later replay. Returning to that terminal can show stale or incomplete output because TerminalPane depends on terminalLiveChunks for incremental xterm updates, and existing grid tiles are not rehydrated when toggleGridView flips the grid back on. Please keep a bounded buffer for hidden terminals, or mark them dirty and force a fresh preview/rehydration before rendering them again.
Artifacts
Repro: focused hidden terminal PTY drop harness
- Contains supporting evidence from the run (text/javascript; charset=utf-8).
Stack trace captured during the T-Rex run
- Keeps the raw stack trace available without making the summary code-heavy.
Repro: Vitest-focused hidden terminal PTY drop test attempted
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
Repro Lightbox Autoplay Poster
- Shows the rendered state that T-Rex checked.
▶ repro-lightbox-autoplay-video.webm
- Shows the flow or interaction that T-Rex exercised.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/ade-cli/src/tuiClient/app.tsx
Line: 6970-6977
Comment:
**Hidden terminals lose output**
This branch drops `pty_data` events for terminal sessions unless they are currently active or visible in the grid. When a Claude Code terminal keeps running after the user switches to another chat, lane, or hides a saved grid, its live output is discarded instead of retained for later replay. Returning to that terminal can show stale or incomplete output because `TerminalPane` depends on `terminalLiveChunks` for incremental xterm updates, and existing grid tiles are not rehydrated when `toggleGridView` flips the grid back on. Please keep a bounded buffer for hidden terminals, or mark them dirty and force a fresh preview/rehydration before rendering them again.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+271
to
+280
| <video | ||
| key={demo.video} | ||
| src={demo.video} | ||
| poster={demo.poster} | ||
| autoPlay | ||
| loop | ||
| controls | ||
| playsInline | ||
| className="block max-h-[78vh] w-full rounded-[4px] border border-white/10 bg-black shadow-[0_40px_120px_-40px_rgba(0,0,0,0.9)]" | ||
| /> |
There was a problem hiding this comment.
The expanded demo video uses autoPlay, but unlike the tile videos it is not muted. Browsers commonly block autoplay for media that may have audio, so clicking a demo can open the lightbox with a poster or blank frame instead of starting playback. Add muted here as well, or remove autoPlay and require the user to press play.
Suggested change
| <video | |
| key={demo.video} | |
| src={demo.video} | |
| poster={demo.poster} | |
| autoPlay | |
| loop | |
| controls | |
| playsInline | |
| className="block max-h-[78vh] w-full rounded-[4px] border border-white/10 bg-black shadow-[0_40px_120px_-40px_rgba(0,0,0,0.9)]" | |
| /> | |
| <video | |
| key={demo.video} | |
| src={demo.video} | |
| poster={demo.poster} | |
| autoPlay | |
| muted | |
| loop | |
| controls | |
| playsInline | |
| className="block max-h-[78vh] w-full rounded-[4px] border border-white/10 bg-black shadow-[0_40px_120px_-40px_rgba(0,0,0,0.9)]" | |
| /> |
Artifacts
Repro: Playwright script that opens the real FeatureGrid lightbox and evaluates the expanded video
- Contains supporting evidence from the run (text/javascript; charset=utf-8).
- Keeps the command output available without making the summary code-heavy.
▶ Screen recording from the T-Rex run
- Shows the flow or interaction that T-Rex exercised.
Repro Lightbox Autoplay Poster
- Shows the rendered state that T-Rex checked.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/src/components/editorial/FeatureGrid.tsx
Line: 271-280
Comment:
**Lightbox autoplay fails**
The expanded demo video uses `autoPlay`, but unlike the tile videos it is not muted. Browsers commonly block autoplay for media that may have audio, so clicking a demo can open the lightbox with a poster or blank frame instead of starting playback. Add `muted` here as well, or remove `autoPlay` and require the user to press play.
```suggestion
<video
key={demo.video}
src={demo.video}
poster={demo.poster}
autoPlay
muted
loop
controls
playsInline
className="block max-h-[78vh] w-full rounded-[4px] border border-white/10 bg-black shadow-[0_40px_120px_-40px_rgba(0,0,0,0.9)]"
/>
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two workstreams. (Scope corrected — this lane does not touch chat persistence,
lanes.suggestName, multi-question approvals, or iOS; those were phantom deletions from diffing against a drifted localmain.)ADE Code (TUI) — kill drawer flicker
ChatRowrenders the expanded + compact chat lists identically; drop theCHATSheader and timestamps.stableInkViewportRowsreserves a row to avoid Ink-5 full-screen clears; terminal-preview frame dedup (sameTerminalPreviewFrame); visible-only PTY buffering (shouldBufferPtyDataForSession) with bounded flush rates.draft:N); simplified cross-terminal mouse baseline; modified-key word-backspace; gridTabescapes to side panes.Web — marketing refresh
FeatureGrid/ShipShowcase; newOgImage/_ogcard;vercel.jsoncache headers for images/videos; brand + demo media assets.Ran
/quality+/test: 716 TUI tests green, web typecheck clean. Will syncmain(which advanced past this lane's fork point) — keeping main's recent work and dropping this lane's now-stale macOS-VM badge code per main'sRemove macOS VM support.Greptile Summary
/_ogpreview route.Confidence Score: 2/5
Merge safety is reduced by runtime regressions in terminal buffering and grid resizing, plus a smaller marketing lightbox playback issue.
The main concerns affect visible ADE Code terminal behavior during common navigation and constrained-layout flows, so the branch should be fixed before merging.
apps/ade-cli/src/tuiClient/app.tsx and apps/web/src/components/editorial/FeatureGrid.tsx
What T-Rex did
Comments Outside Diff (1)
apps/ade-cli/src/tuiClient/app.tsx, line 13637-13650 (link)This resize effect returns when the grid cannot render multiple tiles, but the single-view resize effect also returns while
gridViewActiveis true. In a one-tile grid, or on a small terminal whereMultiChatGridfalls back to rendering one tile, no code resizes the terminal PTY to the tile size. The terminal can keep its previous dimensions, which makes wrapping and preview rows wrong for the visible tile.Artifacts
Repro: focused resize-effect harness
Repro: failing harness output with expected and observed resize calls
Repro: focused Vitest harness attempted before falling back to Node execution
Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(ci+review): commit README hero asset..." | Re-trigger Greptile