aromajoin / material-showcase-ios

✨ An elegant way to guide your beloved users in iOS apps - Material Showcase.
https://aromajoin.com/
Other
363 stars 128 forks source link

Secondary text have multi line draw over target view #92

Open fadizant opened 5 years ago

fadizant commented 5 years ago

When the secondary text have multi line ( more than 3 lines ) and instructionView will show above of target view the text will draw over target view.

I have make some changes in MaterialShowcase.swift to calculate the height depends of text font and size for primary text and secondary text not to be static as before LABEL_DEFAULT_HEIGHT * 2

can you check this issue please, thanks.


private func addInstructionView(at center: CGPoint) {
 .....

 width = containerView.frame.width - (xPosition + xPosition)

 yPosition = center.y - TEXT_CENTER_OFFSET - ( calculateHeight(text: primaryText, font: primaryTextFont ?? UIFont.boldSystemFont(ofSize: primaryTextSize), width: width) +  calculateHeight(text: secondaryText, font: secondaryTextFont ?? UIFont.systemFont(ofSize: secondaryTextSize), width: width) )//LABEL_DEFAULT_HEIGHT * 2
 .....
 }
zahariadaniel16 commented 5 years ago

I agree with this change because I also need it. Developer, please review this issue!

quangctkm9207 commented 5 years ago

@fadizant. Thank you for addressing the issue. Can you send a PR for that?

mjenj commented 4 years ago

@fadizant @quangctkm9207 Has this been addressed at all?

Zyb3r commented 4 years ago

Also having issues with this, can someone please assist? image

quangctkm9207 commented 4 years ago

Can you try to adjust showcase.backgroundRadius to see if the situation can be improved?

mjenj commented 4 years ago

Adjusting the size of the background radius did not help me. Going off what @fadizant said I've created a pull request which dynamically calculates the height https://github.com/aromajoin/material-showcase-ios/pull/133

Please could you take a look @quangctkm9207 ? Thanks

quangctkm9207 commented 4 years ago

Thank you @mjenj. I am going to check it out.

quangctkm9207 commented 4 years ago

@Zyb3r @mjenj : Have you tried the latest version 0.7.2 that has just been released yesterday?

mjenj commented 4 years ago

I just updated now, and the problem is better but unfortunately still not fixed. The size of the target is definitely a factor, and if I make the target smaller, I can see the text, but not the whole target which isn't ideal

Zyb3r commented 4 years ago

As @mjenj mentioned, we updated and it is slightly better, but the text still extends outside of the view

quangctkm9207 commented 4 years ago

Thanks for confirming. I have tried @mjenj PR, and I have a comment over there. It seems that there is no a perfect solution for this issue.

mjenj commented 4 years ago

Thanks, I'll investigate and see if I can find a compromise

mjenj commented 4 years ago

I've added a new comment on the pull request. I agree your solution is working better in most cases, I'm just not sure how to fix it. The 2 factors I believe are the targetHolderRadius and whether the button is on the left or right

mjenj commented 4 years ago

We've found a work around for our use case, by setting the target to 0, however if you manage to solve the problem please ping me. Thanks for being responsive :)

quangctkm9207 commented 4 years ago

Thank you @mjenj.