Use next edge when preferred edge rect would be out of bounds #132
Conversation
This was actually the intended behavior. The arrow is centered under the common section of the axis. Instead of changing this, can we add a property that's similar to |
| @@ -70,7 +70,7 @@ - (IBAction)showRBLPopover:(id)sender { | |||
| } else { | |||
| NSView *button = sender; | |||
| self.RBLPopover.behavior = self.behavior; | |||
| [self.RBLPopover showRelativeToRect:CGRectZero ofView:button preferredEdge:(CGRectEdge)self.preferredEdge]; | |||
| [self.RBLPopover showRelativeToRect:button.bounds ofView:button preferredEdge:(CGRectEdge)self.preferredEdge]; | |||
This is to move the tip of the arrow closer to the actual button in this case? I'd been meaning to fix that up. ✨ |
| @@ -289,7 +302,11 @@ - (void)showRelativeToRect:(CGRect)positioningRect ofView:(NSView *)positioningV | |||
|
|
|||
| if (self.shown) { | |||
| if (self.backgroundView.popoverEdge == popoverEdge) { | |||
| CGSize size = [self.backgroundView sizeForBackgroundViewWithContentSize:contentViewSize popoverEdge:popoverEdge]; | |||
| self.backgroundView.frame = (NSRect){ .size = size }; | |||
| self.backgroundView.popoverEdge = popoverEdge; | |||
There was a problem hiding this comment.
self.backgroundView.popoverEdge is already set since it's in the if statement.
Is there a reason we need to set the frame here?
There was a problem hiding this comment.
👍 about the -popoverEdge.
If we don't set the frame here, but we change the popover's -contentSize before calling this method, the frame of the -popoverWindow is recalculated, but the clipping path isn't updated.
There was a problem hiding this comment.
Hmm… that's a little strange because I'd expect everything to update when you set the contentSize. You shouldn't need to call showRelativeToRect:ofView:preferredEdge: to get the popover to use a new content size.
There was a problem hiding this comment.
I agree but contentSize is just a property with no effects right now. We can make that change in a subsequent pull request if that's 🆒 with you.
| CGFloat minMidpointX = floor(minX + RBLPopoverBackgroundViewBorderRadius); | ||
| CGFloat maxMidpointX = floor(maxX - RBLPopoverBackgroundViewBorderRadius); | ||
| CGFloat minMidpointY = floor(minY + RBLPopoverBackgroundViewBorderRadius); | ||
| CGFloat maxMidpointY = floor(maxY - RBLPopoverBackgroundViewBorderRadius); |
There was a problem hiding this comment.
I think these would benefit from clearer names. They're describing the center points of the arcs. Midpoint doesn't really communicate that to me, but I don't have any great suggestions.
There was a problem hiding this comment.
Arcpoint? Centerpoint?
There was a problem hiding this comment.
How about Centerpoint + a short comment explaining what they represent?
|
🌌 |
|
🚑 |
Use next edge when preferred edge rect would be out of bounds
This fixes a bug where defaulting to the preferred edge when the popover's proposed rect isn't contained in the screen's visible frame, causes the popover to display incorrectly.
Before:
After:
Also fixes a bug where a zero anchor point doesn't render the arrow on the edge.
Before:
After:
