Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Setting a background color for RBLPopover view works without flickering #49

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants

gekitz commented Nov 9, 2012

No description provided.

@jspahrsummers jspahrsummers commented on an outdated diff Nov 9, 2012

Rebel/RBLPopover.m
@@ -257,6 +257,10 @@ - (void)showRelativeToRect:(CGRect)positioningRect ofView:(NSView *)positioningV
}
self.backgroundView = [self.backgroundViewClass backgroundViewForContentSize:contentViewSize popoverEdge:popoverEdge originScreenRect:screenPositioningRect];
+
+ if (self.backgroundColor) {
@jspahrsummers

jspahrsummers Nov 9, 2012

Contributor

Can you please fix the indentation on these lines (tabs, not spaces) and also change this condition to be an explicit comparison against nil?

Contributor

jspahrsummers commented Nov 9, 2012

Looks good to me. @dannygreg, any thoughts?

@dannygreg dannygreg was assigned Nov 9, 2012

Contributor

jspahrsummers commented Nov 25, 2012

@dannygreg Looks like this slipped under the radar. Do you mind re-reviewing it?

Wow, this did indeed slip under my radar. I'll take a look.

I have one problem with this, which is right now the responsibility for appearance is solely that of the background view and I don't really like the idea of starting to muddy the waters now with the background colour being duplicated in RBLPopover itself. Not to mention that, in this implementation, changing the colour after the popover has been opened would have no effect.

I would prefer small tweaks to the appearance like this were handled with a light subclass and using the backgroundViewClass property, it's only a few lines. That said, I can understand that that may feel like overkill in this case, in which case a better fix would be to ensure the background view is initialised when backgroundView is called (much like -view in NSViewController) and not just on showing the popover. That would solve your "flickering" issue without muddying the API.

Contributor

mdiep commented Feb 6, 2014

The background color can now be set directly on the view. (#120)

@mdiep mdiep closed this Feb 6, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment