Skip to content
This repository has been archived by the owner on Jan 16, 2021. It is now read-only.

Adding accessibility identifiers to PFLogInView and PFSignUpView #214

Merged
merged 1 commit into from
Dec 10, 2015
Merged

Adding accessibility identifiers to PFLogInView and PFSignUpView #214

merged 1 commit into from
Dec 10, 2015

Conversation

huguesbr
Copy link
Contributor

@huguesbr huguesbr commented Dec 8, 2015

Adding accessibility identifiers for easier UI Automation Testing.

@nlutsenko
Copy link
Contributor

Whee! This is great! Didn't know someone actually cares about ui testing our components. 😁
Nitpicky review is coming, so bare with me through the process, but I would love to merge it when it's done.

nlutsenko
nlutsenko reviewed Dec 8, 2015
View reviewed changes
ParseUI/Classes/LogInViewController/PFLogInView.h

@see PFLogInView
*/
extern NSString * const PFLogInViewUsernameFieldAccessibilityIdentifier;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Per our style - could you remove the space between * and const, as the star should only be space separated on the left.

@nlutsenko
Copy link
Contributor

Ok, that's about it as far as I can see right now.
The nits apply to the entire PR, meaning that there are more places with the same issue, but I left it out to avoid pollution. Let me know if any of them don't quite make sense or if there is anything I can help you with.

@huguesbr
Copy link
Contributor Author

huguesbr commented Dec 8, 2015

@nlutsenko done! thx for the thorough review

@facebook-github-bot
Copy link

@huguesbr updated the pull request.

@huguesbr huguesbr mentioned this pull request Dec 8, 2015
Merged
@huguesbr
Copy link
Contributor Author

huguesbr commented Dec 9, 2015

@nlutsenko anything else, it should be PR ready..!

nlutsenko
nlutsenko reviewed Dec 9, 2015
View reviewed changes
ParseUI/Classes/LogInViewController/PFLogInView.m
@@ -35,6 +35,18 @@
static NSString *const PFLogInViewDefaultFacebookButtonImageName = @"facebook_icon.png";
static NSString *const PFLogInViewDefaultTwitterButtonImageName = @"twitter_icon.png";

///--------------------------------------
#pragma mark - Accessibility Identifiers
///--------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Add line break before line 41 per overall code style.

@nlutsenko
Copy link
Contributor

Few missing/extra line breaks and will merge it in.

@facebook-github-bot
Copy link

@huguesbr updated the pull request.

@huguesbr
Copy link
Contributor Author

@nlutsenko all done.. wish there was a good linter for objc-c... thx

@nlutsenko
Copy link
Contributor

We are using a clang-format file, but it can't quite catch these things...
So for the time being I work part-time as a linter. 😁

@nlutsenko
Copy link
Contributor

Last piece - could you squash all commits into a single one? I will merge it straight after.

@huguesbr
Copy link
Contributor Author

@nlutsenko done

@nlutsenko
Copy link
Contributor

Whee!

nlutsenko added a commit that referenced this pull request Dec 10, 2015
@nlutsenko
Merge pull request #214 from huguesbr/accessibility
fd9f852 
Adding accessibility identifiers to PFLogInView and PFSignUpView
@nlutsenko nlutsenko merged commit fd9f852 into parse-community:master Dec 10, 2015
@nlutsenko nlutsenko self-assigned this Dec 10, 2015
@nlutsenko nlutsenko added this to the 1.1.8 milestone Dec 10, 2015
@huguesbr
Copy link
Contributor Author

@nlutsenko thx for playing linter...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed GH Review: accepted
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@huguesbr @nlutsenko @facebook-github-bot