Skip to content

fix: Popover positioning #111

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

Open
userFortyTwo wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: Popover positioning #111

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
28ff8fe
Fix popover positioning (#103, deleted)
Feb 24, 2026
851f4d6
Remove comments and fix indentation (#103, #111)
Mar 29, 2026
4b60319
Merge branch 'main' into main
minikin Mar 30, 2026
0fe4e46
Format popover_position_render_object.dart
Apr 3, 2026
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
37 changes: 24 additions & 13 deletions lib/src/popover_position_render_object.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,30 +95,41 @@ final class PopoverPositionRenderObject extends RenderShiftedBox {

double _horizontalOffset(Size size) {
var offset = 0.0;
const horizontalMargin = 8.0;

if (attachRect.left > size.width / 2 &&
PopoverUtils.physicalSize.width - attachRect.right > size.width / 2) {
offset = attachRect.left + attachRect.width / 2 - size.width / 2;
} else if (attachRect.left < size.width / 2) {
offset = arrowHeight;
final centeredOffset =
attachRect.left + attachRect.width / 2 - size.width / 2;

if (centeredOffset >= horizontalMargin &&
centeredOffset + size.width <=
PopoverUtils.physicalSize.width - horizontalMargin) {
offset = centeredOffset;
} else if (centeredOffset < horizontalMargin) {
offset = horizontalMargin;
} else {
offset = PopoverUtils.physicalSize.width - arrowHeight - size.width;
offset = PopoverUtils.physicalSize.width - horizontalMargin - size.width;
}

return offset;
}

double _verticalOffset(Size size) {
var offset = 0.0;
const verticalMargin = 8.0;

if (attachRect.top > size.height / 2 &&
PopoverUtils.physicalSize.height - attachRect.bottom >
size.height / 2) {
offset = attachRect.top + attachRect.height / 2 - size.height / 2;
} else if (attachRect.top < size.height / 2) {
offset = arrowHeight;
final centeredOffset =
attachRect.top + attachRect.height / 2 - size.height / 2;

if (centeredOffset >= verticalMargin &&
centeredOffset + size.height <=
PopoverUtils.physicalSize.height - verticalMargin) {
offset = centeredOffset;
} else if (centeredOffset < verticalMargin) {
offset = verticalMargin;
} else {
offset = PopoverUtils.physicalSize.height - arrowHeight - size.height;
offset = PopoverUtils.physicalSize.height - verticalMargin - size.height;
Comment on lines 96 to +130

@coderabbitai coderabbitai Bot Mar 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the file under review
head -150 lib/src/popover_position_render_object.dart | tail -60

Repository: minikin/popover

Length of output: 1712

🏁 Script executed:

# Check the PopoverUtils definition
find . -name "popover_utils.dart" -type f

Repository: minikin/popover

Length of output: 93

🏁 Script executed:

# Look at the performLayout method to understand the context
sed -n '40,80p' lib/src/popover_position_render_object.dart

Repository: minikin/popover

Length of output: 1185

🏁 Script executed:

# Check PopoverUtils.physicalSize definition
cat lib/src/utils/popover_utils.dart

Repository: minikin/popover

Length of output: 1436

🏁 Script executed:

# Find which widgets use PopoverPositionRenderObject
rg "PopoverPositionRenderObject" --type dart

Repository: minikin/popover

Length of output: 405

🏁 Script executed:

# Look for the parent widget that creates this render object
rg "createRenderObject" lib/src/ --type dart -A 5

Repository: minikin/popover

Length of output: 889

🏁 Script executed:

# Check for any CompositedTransformFollower or similar positioning
rg "CompositedTransformFollower\|Positioned\|Overlay" lib/src/ --type dart

Repository: minikin/popover

Length of output: 41

🏁 Script executed:

# Look at the full popover_position_render_object.dart to understand the class
wc -l lib/src/popover_position_render_object.dart

Repository: minikin/popover

Length of output: 106

🏁 Script executed:

# Read the full file to understand context
cat lib/src/popover_position_render_object.dart

Repository: minikin/popover

Length of output: 3736

🏁 Script executed:

# Look at the widget that uses this render object
cat lib/src/popover_position_widget.dart

Repository: minikin/popover

Length of output: 1055

🏁 Script executed:

# Check how PopoverPositionWidget is used
rg "PopoverPositionWidget" --type dart -B 2 -A 2

Repository: minikin/popover

Length of output: 1008

🏁 Script executed:

# Look at the popover context to understand the full layout structure
head -100 lib/src/popover_context.dart

Repository: minikin/popover

Length of output: 1612

🏁 Script executed:

# Look at PopoverItem to see how PopoverPositionWidget is used
cat lib/src/popover_item.dart

Repository: minikin/popover

Length of output: 4384

🏁 Script executed:

# Check the Stack and how constraints flow through
rg "Stack\|Dialog\|ConstrainedBox" lib/src/ --type dart -B 2 -A 2

Repository: minikin/popover

Length of output: 41

🏁 Script executed:

# Let's understand the coordinate system - check if there are any tests
find . -name "*test*" -type f | grep -i popover

Repository: minikin/popover

Length of output: 83

🏁 Script executed:

# Look at how Popover is shown - the entry point
rg "showPopover" lib/src/ --type dart -A 10

Repository: minikin/popover

Length of output: 797

🏁 Script executed:

# Search for any documentation or examples
find . -name "*.md" -o -name "example*" | head -10

Repository: minikin/popover

Length of output: 328

🏁 Script executed:

# Let's look at the test file to understand typical usage
cat test/popover_test.dart

Repository: minikin/popover

Length of output: 14106

🏁 Script executed:

# Check the example to see how it's typically used
ls -la example/lib/

Repository: minikin/popover

Length of output: 231

🏁 Script executed:

# Look for any issues or discussions about positioning
find . -name "*.md" -exec grep -l "position\|overlay\|constrain" {} \;

Repository: minikin/popover

Length of output: 73

🏁 Script executed:

# Let's check PopoverRenderShiftedBox to see if it has similar logic
cat lib/src/popover_render_shifted_box.dart | head -100

Repository: minikin/popover

Length of output: 2499

Use the render box bounds for clamping instead of the global screen size.

The new center-first positioning logic is better, but _horizontalOffset() and _verticalOffset() still clamp against PopoverUtils.physicalSize, which is the raw device screen size. When PopoverPositionRenderObject is constrained to bounds smaller than the full screen (e.g., within dialogs or other constrained layouts), this can position the popover outside its actual render bounds. Since performLayout() sets this.size before calculateOffset() runs, the clamping should use the render object's actual bounds instead of a global screen size.

🛠️ Proposed fix 
-  double _horizontalOffset(Size size) {
+  double _horizontalOffset(Size childSize) {
     var offset = 0.0;
     const horizontalMargin = 8.0;
+    final availableWidth = this.size.width;

     final centeredOffset =
-        attachRect.left + attachRect.width / 2 - size.width / 2;
+        attachRect.left + attachRect.width / 2 - childSize.width / 2;

     if (centeredOffset >= horizontalMargin &&
-        centeredOffset + size.width <=
-            PopoverUtils.physicalSize.width - horizontalMargin) {
+        centeredOffset + childSize.width <= availableWidth - horizontalMargin) {
       offset = centeredOffset;
     } else if (centeredOffset < horizontalMargin) {
       offset = horizontalMargin;
     } else {
-      offset = PopoverUtils.physicalSize.width - horizontalMargin - size.width;
+      offset = availableWidth - horizontalMargin - childSize.width;
     }

     return offset;
   }

-  double _verticalOffset(Size size) {
+  double _verticalOffset(Size childSize) {
     var offset = 0.0;
     const verticalMargin = 8.0;
+    final availableHeight = this.size.height;

     final centeredOffset =
-        attachRect.top + attachRect.height / 2 - size.height / 2;
+        attachRect.top + attachRect.height / 2 - childSize.height / 2;

     if (centeredOffset >= verticalMargin &&
-        centeredOffset + size.height <=
-            PopoverUtils.physicalSize.height - verticalMargin) {
+        centeredOffset + childSize.height <=
+            availableHeight - verticalMargin) {
       offset = centeredOffset;
     } else if (centeredOffset < verticalMargin) {
       offset = verticalMargin;
     } else {
-      offset = PopoverUtils.physicalSize.height - verticalMargin - size.height;
+      offset = availableHeight - verticalMargin - childSize.height;
     }

     return offset;
   }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/src/popover_position_render_object.dart` around lines 95 - 129, The
clamping logic in _horizontalOffset(Size size) and _verticalOffset(Size size)
currently uses PopoverUtils.physicalSize (device screen) but should use the
render object's actual bounds set in performLayout; replace references to
PopoverUtils.physicalSize.width/height with this.size.width/height (the render
object's size) when checking and computing the max offset and fallback positions
so the popover is clamped to the render object's bounds rather than the full
screen.

}

return offset;
}
}
Loading