-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Install screen version update #448
Conversation
The new install screen will now show up for iOS versions from 15.0 onwards
Changed deployment target to reflect recent changes to the app
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
|
Quick check on this, @Jag-Marcel :) I remember we discussed this, but are we sure we want to increase the minimum version here? We could leave it at 13 and just use the old one from 15 on. The way that it's set up here, we have a minimum of 15 and check if 15 is available. I think keeping it at 13 might be best so that it can still be available on as many levels of devices as possible, but maybe I'm also forgetting another reason why we said 15. Thanks so much for looking into this! |
@andrewtavis Ah, I wasn't sure how it worked, I didn't know it would lock out older versions from downloading it. I don't think there was anything that necessarily required iOS 15 to function, but there were things like the buttons with the info icon that look much better on 15. |
This is some great investigating, @Jag-Marcel! Yes, let's remove the availability checks. Also feel free to remove the longer install directions, which is great as the new ones will be much easier to localize 😊 |
Hmmmm, but let me merge in the other PR and then you can bring in the changes on main into here? All kinds of conflicts if not. Will do that tonight! |
Other PR and the changelog update for it are done, @Jag-Marcel :) Let me know if you have any questions here! |
Without ScribeColors before it, just the key would cause an "Ambiguous use of '.key'" error in the compiler. I assume this was something changed for iOS 14 or 15, the code won't compile unless you specify the used colour library.
- iOS deployment target in project.pbxproj is now correctly set for all keyboards. - Simplified code that doesn't require the #available tag for the new target
titleEdgeInsets seems to have been deprecated since 15.0, Xcode said it would be ignored in it so it seemed like a good idea to remove it.
Changes are done, it's a lot simpler now in places! |
var keyCharColor = UIColor(.keyChar) | ||
var specialKeyColor = UIColor(.keySpecial) | ||
var keyPressedColor = UIColor(.keyPressed) | ||
var keyColor = UIColor(ScribeColor.key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for also going through and changing color assignments! I agree that it makes sense for us to be explicit with the source :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for handling all of this, @Jag-Marcel :) Really great to have the care put into this and allow us to move on from so many of the old version checks 😊
And re the above: let's not focus on this in the short term, but you'd be welcome to make an issue for this :) Sounds like a medium good first issue :) |
Will do 👍 |
Description
Changes the installation screen's description to use the new one from iOS 15.0 onwards, instead of from 17.2. Also changed the deployment target to 15.0 to reflect changes in this PR and in #415.