[WEB-4746] fix: position peek view relative to app layout#7635
[WEB-4746] fix: position peek view relative to app layout#7635sriramveeraghanta merged 1 commit intopreviewfrom
Conversation
WalkthroughThis PR adjusts peek/modal positioning and portal rendering, integrates isPeekView from useAnalytics into modal behavior, and updates updateIsEpic invocation. It also exposes isPeekView from the analytics store hook. Non-embedded peek views and analytics modals now render via portals more broadly with revised absolute positioning and sizing. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Modal as Analytics Work-Items Modal
participant Hook as useAnalytics
participant Store as Analytics Store
participant Portal as Portal Container
User->>Modal: Open modal (peek/full-screen)
Modal->>Hook: useAnalytics()
Hook-->>Modal: { isPeekView, updateIsEpic, ... }
Note over Modal,Hook: On effect
Modal->>Hook: updateIsEpic(isPeekView ? (isEpic ?? false) : false)
alt portalContainer exists
Modal->>Portal: Render modal content
else
Modal->>Modal: Render inline
end
sequenceDiagram
actor User
participant Peek as Issue Peek View
participant Portal as Portal Root
User->>Peek: Open peek (embedIssue? / peekMode)
alt embedIssue == false
Peek->>Portal: Render overlay via portal
Note right of Portal: Absolute positioned<br/>top-0 right-0 bottom-0<br/>w-full/md:w-1/2
else
Peek->>Peek: Render inline
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
apps/web/core/components/issues/peek-overview/view.tsx (3)
116-116: Tailwind nit: prefer md:w-1/2 over md:w-[50%]Functionally identical, but md:w-1/2 avoids arbitrary values and stays consistent with the rest of the codebase.
- "top-0 bottom-0 right-0 w-full md:w-[50%] border-0 border-l": peekMode === "side-peek", + "top-0 bottom-0 right-0 w-full md:w-1/2 border-0 border-l": peekMode === "side-peek",
118-118: Remove duplicate “absolute” in the full-screen variantThe container is already absolute in the base branch. Keeping it twice is harmless but noisy.
- "inset-0 m-4 absolute": peekMode === "full-screen", + "inset-0 m-4": peekMode === "full-screen",
122-122: Portaling all non-embedded views: confirm outside-click/escape semantics and stackingRendering via portal for all non-embedded cases is a good unification. Please sanity-check:
- Outside click still works with the portaled node.
- ESC closes reliably.
- No z-index collisions with analytics modal (both are z-[25] now).
Manual checks:
- Open side-peek and modal modes, verify close-on-outside-click/ESC.
- Open analytics modal simultaneously and confirm expected stacking order between the two.
If we need deterministic stacking, we can bump one to z-[26] or centralize overlay z-indices in a theme token.
apps/web/core/components/analytics/work-items/modal/index.tsx (3)
32-33: Gate updateIsEpic by isOpen and reset on unmountAvoid toggling analytics state when the modal isn’t visible and ensure cleanup to prevent “sticky” epic state.
- useEffect(() => { - updateIsEpic(isPeekView ? (isEpic ?? false) : false); - }, [isEpic, updateIsEpic, isPeekView]); + useEffect(() => { + if (!isOpen) return; + updateIsEpic(isPeekView ? (isEpic ?? false) : false); + return () => updateIsEpic(false); + }, [isOpen, isEpic, isPeekView, updateIsEpic]);
40-40: Anchor the absolute wrapper with inset-0 to avoid layout driftabsolute without offsets relies on the element’s static position; add inset-0 so the wrapper consistently spans the portal container.
- <div className={cn("z-[25] h-full w-full overflow-y-auto absolute")}> + <div className={cn("z-[25] h-full w-full overflow-y-auto absolute inset-0")}>
42-44: Responsive sizing: keep full width on small screensUsing w-1/2 at all breakpoints can make the modal cramped on mobile. Suggest w-full on small, md:w-1/2 on medium+.
- className={`top-0 right-0 z-[25] h-full bg-custom-background-100 shadow-custom-shadow-md absolute ${ - fullScreen ? "w-full p-2" : "w-1/2" - }`} + className={`top-0 right-0 z-[25] h-full bg-custom-background-100 shadow-custom-shadow-md absolute ${ + fullScreen ? "w-full p-2" : "w-full md:w-1/2" + }`}If desired, we can also cap width on very large screens (e.g., max-w-[960px]) without changing behavior elsewhere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/core/components/analytics/work-items/modal/index.tsx(3 hunks)apps/web/core/components/issues/peek-overview/view.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and lint web apps
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/web/core/components/issues/peek-overview/view.tsx (1)
113-113: Absolute positioning: confirm full-viewport context for#full-screen-portalThe portal element in question now renders as:
<div id="full-screen-portal" className="inset-0 absolute w-full" />Tailwind’s
inset-0sets top/right/bottom/left to0, but without an explicit height (h-full), its height will collapse to the height of its positioned ancestor. In your layout the immediate ancestor is:<div className="relative flex flex-col h-full w-full …"> <div id="full-screen-portal" className="inset-0 absolute w-full" /> … </div>That ancestor has
h-full, but neither<html>nor<body>in your root layout carryh-full(and there’s no global CSS enforcingheight:100%). As a result, the portal will fill only as much height as the content wrapper hierarchy provides—and may clip if that container ever shrinks or scrolls.To guarantee true full-viewport coverage (and avoid subtle clipping or misalignment), please verify one of the following approaches:
- Switch the portal to fixed positioning at the viewport level:
- <div id="full-screen-portal" className="inset-0 absolute w-full" /> + <div id="full-screen-portal" className="fixed inset-0 z-[25]" />- Add an explicit height utility so it spans its container’s full height:
- className="inset-0 absolute w-full" + className="inset-0 absolute w-full h-full"- Ensure that all parent wrappers (including
<html>,<body>, your root<AppLayout>and theAuthenticationWrapper/WorkspaceAuthWrapper) propagateh-fullso theh-fullon your flex container truly reaches the viewport.- Alternatively, move the portal out of the nested layout entirely (e.g. render it in the root layout) so its positioning context is unambiguously the viewport.
Please confirm which pattern you’d prefer, or that the existing hierarchy already enforces 100% height through every wrapper.
apps/web/core/components/analytics/work-items/modal/index.tsx (1)
23-23: No action needed:isPeekViewis correctly exposed byuseAnalyticsI’ve confirmed that:
- The
useAnalyticshook (inapps/web/core/hooks/store/use-analytics.ts) returns theIAnalyticsStore, which includes theisPeekViewboolean property and its updaterupdateIsPeekView.- The store interface and implementation (
apps/web/core/store/analytics.store.ts) declare and initializeisPeekView: boolean = false, and provide theupdateIsPeekView(isPeekView: boolean)action.- Downstream components (including
WorkItemsModaland its content) correctly destructureisPeekView(and where needed,updateIsPeekView) fromuseAnalytics.Everything is wired and typed as expected—no changes required here.
2 participants
Description
Type of Change
Summary by CodeRabbit
Bug Fixes
Refactor