Bubble padding is no longer working in version 2.3.1

Question

Bubble padding is no longer working in version 2.3.1

Closed this issue 7 years ago · 14 comments

I just replaced version 2.3.0 by 2.3.1 and compiled my existing project in Xcode 8.3.3.
The bubble is configured with sidePadding = 10.0.

When running it on an iPhone 7 with iOS 11.0.3 there is a big difference in padding.

Version 2.3.0

Version 2.3.1

The padding is no longer present.
Please advise.

Answer 1 · 2017-11-09T07:38:12.000Z

i am hitting it also.

Answer 2 · 2017-11-09T08:12:56.000Z

The Release Notes of 2.3.1 are incomplete by only referring to:

Fixes issue in iOS 11 where tooltip would not display when presenting via a UIBarButtonItem.

When I look at the master, 2.3.0 was commit 176, while 2.3.1 is 199. So there are many more changes in 2.3.1.
Commits 185 and 186 refer to a new property for padding.
The change log of commit 186 c9ab6ae shows indeed new property shouldEnforceCustomViewPadding.
I see it's never initialised, except in initWithCustomView: which we don't use.

We have reverted back to 2.3.0.
@TingtingAn Can you try with shouldEnforceCustomViewPadding = YES?

Answer 3 · 2017-11-10T09:21:36.000Z

i have never to use initWithCustomView also

Answer 4 · 2017-11-12T06:45:07.000Z

Using sidePadding = 10.0 and shouldEnforceCustomViewPadding = YESworked for me in 2.3.1

Answer 5 · 2017-11-12T10:00:25.000Z

@pventura1976 Thanks for confirming this.
This seems to be a bug.
Why is it required when setting property sidePadding to explicitly set shouldEnforceCustomViewPadding?
At least there is a work-around now, but it doesn't make sense.

Answer 6 · 2017-11-17T18:39:45.000Z

Hey @FungusHumungus @pventura1976 I'm really sorry for the mistake. Perhaps I should have set shouldEnforceCustomViewPadding and sidePadding to be on by default in v2.3.2. What do you think?

Answer 7 · 2017-11-17T20:39:26.000Z

@kleinlieu Thanks for your reaction. Before jumping to a solution; what is the purpose of new property shouldEnforceCustomViewPadding?

I would say that if a user wants to have padding, he defines sidePadding.
If he uses property customView and wants padding, he defines both customView and sidePadding.
For me it's not clear what shouldEnforceCustomViewPadding should control.

It seems to be some unnecessary BOOL introduced for the custom view.
But in my opinion it can be completely deleted.
It is now used as IF condition in - (CGRect)contentFrame and - (void)presentPointingAtView:(UIView *)targetView inView:(UIView *)containerView animated:(BOOL)animated.
The IF condition can be replaced by:
if (_sidePadding > 0) {

Do you agree?

UPDATE This probably won't work, as - (CGRect)contentFrame uses bubblePaddingX and bubblePaddingY, see my next comment. But then I don't understand why the padding works properly for @pventura1976 when shouldEnforceCustomViewPadding is set to YES.

Answer 8 · 2017-11-17T20:45:13.000Z

Another over dimension seems to be the parameters bubblePaddingX and bubblePaddingY. What is the difference with sidePadding?

It would really help if all (less obvious) properties could have proper documentation in the header file. I bet that this would have prevented code implementation issues like this one. Because now there is clearly a misunderstanding of the meaning/definition of some properties.
I'm happy to assist with this, but at least someone needs to explain these properties.

Answer 9 · 2017-11-17T20:50:24.000Z

BTW, sidePadding is already set to a default value of 2.0 in - (id)initWithFrame:(CGRect)frame.

Answer 10 · 2017-11-17T22:32:02.000Z

I like this proposal Best. Pedro Ventura Gómez El 17 nov 2017 21:50 +0100, funnel20 <notifications@github.com>, escribió:

…

BTW, sidePadding is already set to a default value of 2.0 in - (id)initWithFrame:(CGRect)frame. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Answer 11 · 2017-11-18T12:15:32.000Z

As mentioned before, we need to know the functionality first.
I figured it out by playing around with the DEMO project.

IMPORTANT
I was able to change sidePadding in the default DEMO project, where shouldEnforceCustomViewPadding is disabled.
In order to see the proper effect of cornerRadius, bubblePaddingX and bubblePaddingY I had to enable shouldEnforceCustomViewPadding.
Hence I changed the title if this issue from Property "sidePadding" to Bubble padding.
It's worth noting that I was mistaking sidePadding with the inner bubble padding, i.e. the offset between the text and the bubble boundary. After playing around with the DEMO project I learned that this bubble padding is controlled by cornerRadius, bubblePaddingX and bubblePaddingY.

There are 5 properties playing a role:

sidePadding
This is the horizontal offset between the bubble and the view boundary:

The default value is 2. In the above screenshot it was set to 10.
cornerRadius
This will round the corners:

But it also increases the bubble around the text. See the difference with the previous image when increasing the value:

The default value is 10.
bubblePaddingX

This is the horizontal offset between the bubble text and the bubble boundary:
IMPORTANT: When cornerRadius >0 it's added to bubblePaddingX, so the horizontal offset is the total of both properties.
The default value is 0. When this value >0, the horizontal bubble size increases.
bubblePaddingY
This is the vertical offset between the bubble text and the bubble boundary:

IMPORTANT: When cornerRadius >0 it's added to bubblePaddingY, so the vertical offset is the total of both properties.
The default value is 0. When this value >0, the vertical bubble size increases.
shouldEnforceCustomViewPadding
This seems to be an unnecessary internal property which currently causes this issue that bubble padding is not working.

Answer 12 · 2017-11-18T12:23:38.000Z

SOLUTION
Now that the functionality of all involved properties is clear, the solution seems simple:

Remove property shouldEnforceCustomViewPadding
In - (CGRect)contentFrame, the complete IF-ELSE can removed, as the IF statement should always be performed, as it contains all the 3 discussed properties: cornerRadius, bubblePaddingX and bubblePaddingY:

- (CGRect)contentFrame {
    CGRect bubbleFrame = [self bubbleFrame];
    
    CGRect contentFrame = CGRectMake(bubbleFrame.origin.x + _cornerRadius + _bubblePaddingX,
                                     bubbleFrame.origin.y + _cornerRadius + _bubblePaddingY,
                                     bubbleFrame.size.width - (_bubblePaddingX*2) - (_cornerRadius*2),
                                     bubbleFrame.size.height - (_bubblePaddingY*2) - (_cornerRadius*2));
    return contentFrame;
}

Similar in - (void)presentPointingAtView:(UIView *)targetView inView:(UIView *)containerView animated:(BOOL)animated the IF condition should always be performed, so remove IF-ELSE:

_bubbleSize = CGSizeMake(textSize.width + (_bubblePaddingX*2) + (_cornerRadius*2), textSize.height + (_bubblePaddingY*2) + (_cornerRadius*2));

Verification
I have verified the DEMO project after these 3 changes and now the bubble padding does properly work without changing anything to the DEMO code.

Recommendations

Why does cornerRadius automatically apply bubble padding?
This is not common, see for example cornerRadius of CALayer.
If for a larger cornerRadius bubble padding is necessary, this can be configured via bubblePaddingX and bubblePaddingY.
Properly document each property in the header file.
Be accurate with the Release Notes.
The Release Notes of 2.3.1 are incomplete by only referring to an iOS 11 fix for the tooltip on a UIBarButtonItem.
When looking at the master, it appears that there are many more changes in 2.3.1. Even new properties such as the erroneous shouldEnforceCustomViewPadding and it's code that caused this issue.

Answer 13 · 2017-11-18T13:05:42.000Z

I have created a pull request with the solution.
Should I make another with more documentation in the header file?

Answer 14 · 2017-11-27T12:51:59.000Z

This is resolved as 2.3.2