Skip to content
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

Feature/header and keyboard accessibility #19

Closed
wants to merge 7 commits into from
Closed

Feature/header and keyboard accessibility #19

wants to merge 7 commits into from

Conversation

ForeverTangent
Copy link
Contributor

A first take on adding accessibility to just the Header and Keyboard. Not complete functionality for the Header and Keyboard them but I would say about 95%. Just something people can get to know things with. IE you can select a Preset, and play some notes.

I had to clarify some of the descriptions of controls in the Storyboard for future reference, so I hope that isn't a problem. There are also a couple custom controls you guys have created, I have to figure out a way to interact with.

This post can provide a guide how to set up VoiceOver.

@megastep
Copy link
Member

megastep commented Sep 7, 2018

This looks wrong, you probably need to send the PR against the develop branch.

@ForeverTangent ForeverTangent changed the base branch from master to develop September 7, 2018 00:34
@ForeverTangent
Copy link
Contributor Author

ForeverTangent commented Sep 7, 2018 via email

@@ -33,4 +33,4 @@ SPEC CHECKSUMS:

PODFILE CHECKSUM: ee630cf59de150e49c4fb03329e288268269f85e

COCOAPODS: 1.5.3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update your CocoaPods install :)

@@ -6,17 +6,17 @@
<string>development</string>
<key>com.apple.developer.icloud-container-identifiers</key>
<array>
<string>iCloud.com.audiokitpro.AudioKitSynthOne</string>
<string>iCloud.com.audiokitpro.AudioKitSynthOne.Synth</string>
<string>iCloud.com.audiokitpro.AudioKitSynthOne.staque</string>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please exclude this file from the PR since it's just for your own benefit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I just delete the file from the PR or do I have to start a new one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might be able to remove it from the PR in GitHub though I have never tried that myself.

@@ -1423,12 +1423,12 @@
TargetAttributes = {
941645C920B3597300D62851 = {
CreatedOnToolsVersion = 9.3.1;
DevelopmentTeam = 9W69ZP8S5F;
DevelopmentTeam = 69WU6LWMSQ;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also exclude changes to this project file from the PR since it's just your internal configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as Above

@@ -20,7 +20,7 @@
<key>CFBundleDevelopmentRegion</key>
<string>en</string>
<key>CFBundleDisplayName</key>
<string>Synth One</string>
<string>Synth One Develop</string>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also looks unnecessary to be merged


private func setKeyboardModeSegment() {
keyboardModeSegment.subviews[0].accessibilityLabel = "White"
keyboardModeSegment.subviews[1].accessibilityLabel = "Black"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest using NSLocalizedString on all the localizable strings here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OH Good Catch I totally Forgot about those.

@ForeverTangent
Copy link
Contributor Author

Actually how do I kill a PR... I just tried to remove a a file from it and bad things...

@megastep
Copy link
Member

megastep commented Sep 7, 2018

Just close this PR with the button down here, or I can do it for you.

@ForeverTangent ForeverTangent deleted the feature/header-and-keyboard-accessibility branch September 7, 2018 00:51
@ForeverTangent
Copy link
Contributor Author

ForeverTangent commented Sep 7, 2018 via email

@megastep
Copy link
Member

megastep commented Sep 7, 2018

But yeah looks like you deleted the entire project file instead of just the part of the commit

@ForeverTangent
Copy link
Contributor Author

Yeah... that is why I wanted to kill it...

@ForeverTangent
Copy link
Contributor Author

I swear someone cast a Git Curse on me once.

@extreamsd extreamsd mentioned this pull request Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants