-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat(android): Android install page #185
Conversation
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.
Is the keyboard version correct?
keyboards/install.php
Outdated
'name' => $keyboard->name, | ||
'host' => KeymanHosts::Instance()->downloads_keyman_com, | ||
'tier' => $tier, | ||
'version' => $version, |
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.
'version' => $version, | |
'version' => $keyboard->version, |
Since $version
is the Keyman for Android version for the tier, I was getting a non-existent keyboard link
https://downloads.keyman.com/keyboards/sil_ipa/13.0.6218/sil_ipa.kmp
Do we still need $version = self::$versions->android->$tier
if the "Install Keyman for Android" link goes straight to the Play Store?
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.
Good pick! Fixed.
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.
Also fixed on iOS PR #184 and for Linux and macOS pages.
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.
lgtm
Relates to keymanapp/api.keyman.com#109.