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

Expose view controller background colors for styling by host apps #18

Merged
merged 11 commits into from
Aug 7, 2018

Conversation

mindgraffiti
Copy link
Contributor

@mindgraffiti mindgraffiti commented Jul 27, 2018

Fixes #12.

Every view controller in the authentication flow is set to WPStyleGuide.lightGrey in InterfaceBuilder. Need to remove the manual background color assignment, expose the view controller background color property, and set it in both host apps.

To test please see: woocommerce/woocommerce-ios#195

@mindgraffiti mindgraffiti self-assigned this Jul 27, 2018
@mindgraffiti mindgraffiti added the enhancement New feature or request label Jul 27, 2018
@mindgraffiti mindgraffiti added this to the 1.0.5 milestone Jul 27, 2018
@mindgraffiti
Copy link
Contributor Author

@jleandroperez would you please review? this one is ready :octocat:

Copy link
Contributor

@jleandroperez jleandroperez left a comment

Choose a reason for hiding this comment

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

@mindgraffiti Looks great!! Can we please please move the change to the shared base class?. THANK YOU!!!

///
@objc func styleBackground() {
view.backgroundColor = WordPressAuthenticator.shared.style.viewControllerBackgroundColor
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this snippet to the base class?

view.backgroundColor = WordPressAuthenticator.shared.style.viewControllerBackgroundColor
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Same here!

@objc func styleBackground() {
view.backgroundColor = WordPressAuthenticator.shared.style.viewControllerBackgroundColor
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Aaaand same here!

private func styleBackground() {
view.backgroundColor = WordPressAuthenticator.shared.style.viewControllerBackgroundColor
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Aaaand same here!

///
private func styleBackground() {
view.backgroundColor = WordPressAuthenticator.shared.style.viewControllerBackgroundColor
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Aaaand same here!

@objc func styleBackground() {
view.backgroundColor = WordPressAuthenticator.shared.style.viewControllerBackgroundColor
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Aaaand same here!

@objc func styleBackground() {
view.backgroundColor = WordPressAuthenticator.shared.style.viewControllerBackgroundColor
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Aaaand same here!

@mindgraffiti
Copy link
Contributor Author

ready for another look. thank you @jleandroperez +1

Copy link
Contributor

@jleandroperez jleandroperez left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @mindgraffiti !!!

:shipit:

@mindgraffiti mindgraffiti merged commit e0dc960 into develop Aug 7, 2018
@mindgraffiti
Copy link
Contributor Author

thank you Jorge!

@mindgraffiti mindgraffiti deleted the feature/more-configurations branch August 7, 2018 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants