fix: make PermissionShield tooltip placement adaptive to viewport - #1776
fix: make PermissionShield tooltip placement adaptive to viewport#1776dhruveshmishra wants to merge 7 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Permission Shield tooltip now supports responsive boundary padding, typography, dimensions, scrolling, and viewport-aware placement. Callers can override padding per side. Popper uses fallback placements and overflow handling. ChangesPermission tooltip positioning
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
f724f3b to
7be234c
Compare
There was a problem hiding this comment.
Pull request overview
Updates PermissionShield’s tooltip Popper configuration to better avoid viewport clipping and fixed UI chrome (top header / left sidebar) by tuning Popper modifiers and layering.
Changes:
- Switches Popper configuration to use explicit
popperOptions.modifierswith updatedflipfallback order. - Adds viewport-boundary padding (top/left/right/bottom) and disables
tetherforpreventOverflow. - Sets an explicit Popper
zIndexvia inlinestyleto alter how the tooltip layers relative to other fixed UI.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/custom/permissions.tsx`:
- Around line 431-441: Update the tooltip positioning configuration around the
flip and preventOverflow modifiers so narrow viewports can accommodate the
tooltip’s minWidth. Replace the fixed left padding of 270 with responsive or
hidden-sidebar-aware padding, or conditionally make the tooltip width responsive
below the 578px breakpoint, while preserving the existing wide-layout behavior.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e57928f4-f858-4b20-95cb-bc4575dc741d
📒 Files selected for processing (1)
src/custom/permissions.tsx
d812240 to
986603a
Compare
…yer5io#1772) Signed-off-by: dhruveshmishra <dhruveshmishra09@gmail.com>
986603a to
c96f1ef
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Maanvi212006
left a comment
There was a problem hiding this comment.
This is what all I tested:
- Build, lint, and type-check all pass clean on the changed file (
permissions.tsx); no regressions elsewhere. - Full test suite: 493/493 passing
- Verified both bot review comments (Copilot's zIndex risk, CodeRabbit's hardcoded padding) are actually fixed in the current code, not just claimed.
Maanvi212006
left a comment
There was a problem hiding this comment.
The tooltip does not render properly across different viewport sizes
WhatsApp.Video.2026-08-09.at.4.05.07.AM.mp4
…sive Signed-off-by: dhruveshmishra <dhruveshmishra09@gmail.com>
9de096a to
fe678a9
Compare
|
Thanks @Maanvi212006 for your suggestion , I have fixed the responsive problem u can take a look. |
…nd zIndex Signed-off-by: dhruveshmishra <dhruveshmishra09@gmail.com>
0cc9fc1 to
ec8656b
Compare
Signed-off-by: Dhruvesh Mishra <dhruveshmishra09@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/custom/permissions.tsx (4)
127-175: 🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy liftRestore the
PermissionSessionContextdeclaration.Line 127 declares
PermissionShield, but Line 571 declaresPermissionShieldagain. The first declaration also readsdisplayedKeys,subtitle,setOpen, anduniqueIdwithout declaring them in its visible props or scope. Line 617 referencesPermissionSessionContext, but this file does not declare that symbol. Split these components correctly before merge.Also applies to: 571-621
🤖 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 `@src/custom/permissions.tsx` around lines 127 - 175, Restore the PermissionSessionContext declaration and separate the two PermissionShield implementations into their intended component boundaries. Update the first PermissionShield to use only values declared in its props or scope, including displayedKeys, subtitle, setOpen, and uniqueId, while preserving the later component’s PermissionSessionContext usage. Remove or relocate the duplicate declaration so each component and context is defined exactly once.
650-668: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd
left-starttofallbackPlacements. The explicit Popper fallback list replaces the default list, so the tooltip cannot use a left placement unlessleft-startis included.🤖 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 `@src/custom/permissions.tsx` around lines 650 - 668, Update the Popper `flip` modifier configuration in the permissions tooltip to include `left-start` in `fallbackPlacements`, preserving the existing fallback placements and other modifier options.
274-275: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRestore balanced JSX tags.
Biome cannot find corresponding closing tags for
React.FragmentandBoxbeginning on Lines 274-275. The module cannot parse until the JSX tree is corrected.🤖 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 `@src/custom/permissions.tsx` around lines 274 - 275, Complete the JSX tree in the permission-rendering block beginning with React.Fragment and Box by adding the corresponding closing tags in the correct nesting order. Ensure every opened element, including React.Fragment and Box, is closed so the module parses successfully.Source: Linters/SAST tools
181-209: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse Sistent semantic color tokens throughout the permission UI.
The new styles mix MUI palette values with literal white and RGBA colors. In
variant="card",background.papercan be light while the subtitle and key content remain forced to white, which can make permission details unreadable.
src/custom/permissions.tsx#L181-L209: derive card and tooltip palette roles from Sistent theme exports.src/custom/permissions.tsx#L243-L252: use the derived subtitle color instead of literal white.src/custom/permissions.tsx#L313-L338: use derived key and description colors instead of literal white and RGBA values.src/custom/permissions.tsx#L640-L681: replace literal tooltip background, foreground, border, and shadow colors with semantic tokens.As per coding guidelines, “Theme-aware UI must use Sistent theme exports and semantic palette tokens rather than raw MUI defaults.”
🤖 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 `@src/custom/permissions.tsx` around lines 181 - 209, Replace raw MUI palette values and literal white/RGBA colors in the permission UI with Sistent semantic theme tokens. In src/custom/permissions.tsx lines 181-209, derive both card and tooltip palette roles from Sistent theme exports; update lines 243-252 to use the derived subtitle color, lines 313-338 to use derived key and description colors, and lines 640-681 to use semantic tokens for tooltip background, foreground, border, and shadow.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@src/custom/permissions.tsx`:
- Around line 127-175: Restore the PermissionSessionContext declaration and
separate the two PermissionShield implementations into their intended component
boundaries. Update the first PermissionShield to use only values declared in its
props or scope, including displayedKeys, subtitle, setOpen, and uniqueId, while
preserving the later component’s PermissionSessionContext usage. Remove or
relocate the duplicate declaration so each component and context is defined
exactly once.
- Around line 650-668: Update the Popper `flip` modifier configuration in the
permissions tooltip to include `left-start` in `fallbackPlacements`, preserving
the existing fallback placements and other modifier options.
- Around line 274-275: Complete the JSX tree in the permission-rendering block
beginning with React.Fragment and Box by adding the corresponding closing tags
in the correct nesting order. Ensure every opened element, including
React.Fragment and Box, is closed so the module parses successfully.
- Around line 181-209: Replace raw MUI palette values and literal white/RGBA
colors in the permission UI with Sistent semantic theme tokens. In
src/custom/permissions.tsx lines 181-209, derive both card and tooltip palette
roles from Sistent theme exports; update lines 243-252 to use the derived
subtitle color, lines 313-338 to use derived key and description colors, and
lines 640-681 to use semantic tokens for tooltip background, foreground, border,
and shadow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 32ad6797-9f5f-439a-95d4-9cf2645b908f
📒 Files selected for processing (1)
src/custom/permissions.tsx
… sizing and zIndex Signed-off-by: dhruveshmishra <dhruveshmishra09@gmail.com>
Fixes #1772
Summary
This PR fixes the PermissionShield tooltip (the permission warning popup) from getting cut off at the top of the screen or overlapping the fixed top header and left sidebar.
What I changed
Screen recording
Screen.Recording.2026-08-05.at.10.47.09.PM.mov
Responsive:-
Screen.Recording.2026-08-11.at.1.56.04.AM.mov
Summary by CodeRabbit
Improvements