search

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

Fix for positioning instruction view #116 #124

Closed ezefire closed 4 years ago

quangctkm9207 commented 4 years ago

Thank @ezefire for your PR. Merged it already.

ezefire commented 4 years ago

Hi! Thanks for the info! I hope the changes will help.

Will you do a new release with that changes?

El vie., 10 abr. 2020 11:38, Quang Nguyen notifications@github.com escribió:

Thank @ezefire https://github.com/ezefire for your PR. Merged it already.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aromajoin/material-showcase-ios/pull/124#issuecomment-611960378, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDHRYTMYEYK5M5SKNPAAXTRL3SJXANCNFSM4L37A7KA .

quangctkm9207 commented 4 years ago

I am going to release the patch version soon.

quangctkm9207 commented 4 years ago

@ezefire : After the changes in your last PR, when we increase the background radius, it seems that the text is still not centered. If you have time, can you please check it out? Below is the sample app that I run on iPad. image

ezefire commented 4 years ago

Hi,

I've made many tests in the code i forked from your code and effectively i found a problem changing the radius but only on iPad. On iPhone it works normally. I was looking for a bug in the code but i saw that the changes i've done where related to iPhone.

Is it possible that the problem was before my changes or that another PR has broke that ?

I attach many screenshots from iPhone:

El mar., 14 abr. 2020 a las 4:03, Quang Nguyen (notifications@github.com) escribió:

@ezefire https://github.com/ezefire : After the changes in your last PR, now we are not able to change the background radius. It seems that the background radius is fixed at 300. showcase.radius = 1000 // Unable

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aromajoin/material-showcase-ios/pull/124#issuecomment-613185334, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDHRYTLMDSDBTL3ZELM7SLRMPAAJANCNFSM4L37A7KA .

ezefire commented 4 years ago

After more tests i've found that there are no problems in iPad related to the radius. The problem on iPad is the position of the text but not the radius. I add many screenshots:

El mar., 14 abr. 2020 a las 9:42, Cristian Jimenez Rueda (ezefire@gmail.com) escribió:

Hi,

I've made many tests in the code i forked from your code and effectively i found a problem changing the radius but only on iPad. On iPhone it works normally. I was looking for a bug in the code but i saw that the changes i've done where related to iPhone.

Is it possible that the problem was before my changes or that another PR has broke that ?

I attach many screenshots from iPhone:

  • Radius = 400 [image: Screen Shot 2020-04-14 at 09.41.28.png]

  • Radius = 500 [image: Screen Shot 2020-04-14 at 09.38.33.png]

El mar., 14 abr. 2020 a las 4:03, Quang Nguyen (notifications@github.com) escribió:

@ezefire https://github.com/ezefire : After the changes in your last PR, now we are not able to change the background radius. It seems that the background radius is fixed at 300. showcase.radius = 1000 // Unable

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aromajoin/material-showcase-ios/pull/124#issuecomment-613185334, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDHRYTLMDSDBTL3ZELM7SLRMPAAJANCNFSM4L37A7KA .

quangctkm9207 commented 4 years ago

I am sorry that it is not because of your changes. The issue with iPad is still there from the past. https://github.com/aromajoin/material-showcase-ios/issues/116

I have just thought that your PR solved the issue on iPad too.