scribe-org / Scribe-iOS

iOS app with keyboards for language learners
https://apps.apple.com/app/scribe-language-keyboards/id1596613886
GNU General Public License v3.0
131 stars 79 forks source link

Clicking ok on app hint when its over a menu option clicks the menu option 465 #468

Closed fabulouiOS-monk closed 5 months ago

fabulouiOS-monk commented 5 months ago

Contributor checklist


Description

Made the swiftUI view as userInteracable so that the table view's option doesn't get clicked on click of OK on app hints.

NOTE: This is only initial commit to check whether it works or not.

Related issue

github-actions[bot] commented 5 months ago

Thank you for the pull request!

The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and iOS rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you!

Maintainer checklist

andrewtavis commented 5 months ago

As of now it's still not fixed, @fabulouiOS-monk. Here's a screen recoding from a 3rd gen iPhone SE simulator:

https://github.com/scribe-org/Scribe-iOS/assets/24387426/c0509d15-2ff8-4675-a16f-d1b7ac9d6919

So steps above are close it normally, reset the hints, scroll down and then try to close it when it's above the GitHub link, and instead we're taken to GitHub. Let me know if you have some thoughts here!

fabulouiOS-monk commented 5 months ago

@andrewtavis, in the latest commit, pushed the changes for the blink animation.

andrewtavis commented 5 months ago

Hey @fabulouiOS-monk 👋 This is really great! One final comment on this now that I'm able to play around with it: Can we have the tip view show up only if it's at the top of the screen? As of now the condition is that if it's scrolling up, then we show the tip, but then if we scroll down and then scroll up a bit, then it still covers some of the menu options. I think that a conditional that the current scroll position is the default would be the easiest to assure that the tooltip isn't over a menu option :)

Thanks for all the hard work here!

fabulouiOS-monk commented 5 months ago

Ok, will see the fix for it, @andrewtavis!

fabulouiOS-monk commented 5 months ago

@andrewtavis, the latest commit has the fix to show the tip card view only when the user is at the top. Please have a look 😃 !

andrewtavis commented 5 months ago

Looking forward, @fabulouiOS-monk!

andrewtavis commented 5 months ago

Hey @fabulouiOS-monk 👋 Made the speed that the tip view appears a bit faster, just from testing it out. Really wanted to bring it in, but then on testing with an iPhone 15 the functionality isn't working as the value of private var topContentOffset: CGFloat = 116 seems to be only for iPhone SE, so it breaks the functionality and the tip disappears without coming back on larger devices. Where is the value of 116 coming from? Are we able to define this programmatically such that it'll be changed based on device dimensions, etc?

andrewtavis commented 5 months ago

Also a note for myself, if we can figure out programmatically what the location of the About hint view is, then it'd be nice if we could also set that value to the height of hostingController.view.frame in Installation here:

https://github.com/scribe-org/Scribe-iOS/blob/d8663e5e4e189343a1fb242b8bfa894734eb09a1/Scribe/InstallationTab/InstallationVC.swift#L282

I'm realizing that the hint on the installation page is variable based on the height of the device, and the number we're looking for is likely similar for what to set 116 for in topContentOffset and 178 in Installation's hostingController.view.frame. Note that they don't seem to similar, but 116 is based on an iPhone SE, and I chose the value of 178 based on an iPhone 15 Pro Max. There's a good chance that both of these are being set to "What's vertical position of the About tip view relative to the top of the screen?" :)

Let me know on 116 and if you have any thoughts. Once again very close! After this we're done with tip views, I swear 😄

fabulouiOS-monk commented 5 months ago

Hey @fabulouiOS-monk 👋 Made the speed that the tip view appears a bit faster, just from testing it out. Really wanted to bring it in, but then on testing with an iPhone 15 the functionality isn't working as the value of private var topContentOffset: CGFloat = 116 seems to be only for iPhone SE, so it breaks the functionality and the tip disappears without coming back on larger devices. Where is the value of 116 coming from? Are we able to define this programmatically such that it'll be changed based on device dimensions, etc?

Oops! Let me check it. 😅 I was only checking in SE and not others.

andrewtavis commented 5 months ago

All good, @fabulouiOS-monk! Happens :)

fabulouiOS-monk commented 5 months ago

@andrewtavis, pushed the fix for all types of devices. 😄 Hope it all works.