Skip to content
This repository was archived by the owner on Feb 5, 2025. It is now read-only.

Conversation

@jaclync
Copy link
Contributor

@jaclync jaclync commented Jan 14, 2021

Fixes woocommerce/woocommerce-ios#2716 and woocommerce/woocommerce-ios#3386

Why

In NUXButton, the spinner color is set to style.secondaryTitleColor when the button is disabled. The same style.secondaryTitleColor is also used as the button title color when the button is secondary style and disabled - this usage fits its name "secondaryTitleColor" the most. Because of this parameter reuse and the design in WCiOS, the same color secondaryTitleColor results in the spinner color not visible at all in WCiOS (example screenshot).

Changes

This PR added a new parameter WordPressAuthenticatorStyle.disabledButtonActivityIndicatorColor for the spinner color when the button is disabled in NUXButton.

Testing

WPiOS

Please check out this PR wordpress-mobile/WordPress-iOS#15639 - feel free to merge it later, or just test the changes and add the parameter the next time WPiOS updates the authenticator pod

WCiOS

Please check out this PR: woocommerce/woocommerce-ios#3472

Example screenshots

WPiOS

\ before after
log in with email Simulator Screen Shot - iPhone 8 Plus - 2021-01-14 at 17 06 36 Simulator Screen Shot - iPhone 8 Plus - 2021-01-14 at 17 31 07
log in with site address Simulator Screen Shot - iPhone 8 Plus - 2021-01-14 at 17 07 07 Simulator Screen Shot - iPhone 8 Plus - 2021-01-14 at 17 31 18

WCiOS

\ before after
log in with email Simulator Screen Shot - iPhone 8 Plus - 2021-01-14 at 16 50 04 Simulator Screen Shot - iPhone 8 Plus - 2021-01-14 at 17 30 06
log in with site address Simulator Screen Shot - iPhone 8 Plus - 2021-01-14 at 16 51 14 Simulator Screen Shot - iPhone 8 Plus - 2021-01-14 at 17 30 37

@jaclync jaclync self-assigned this Jan 14, 2021
Pod::Spec.new do |s|
s.name = "WordPressAuthenticator"
s.version = "1.33.0"
s.version = "1.33.1-beta.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

We leave the patch version for hotfixes, so this should be 1.34.0-beta.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch! Updated the pod version in: 30084d9

@ScoutHarris ScoutHarris self-requested a review January 14, 2021 18:57
Copy link
Contributor

@ScoutHarris ScoutHarris left a comment

Choose a reason for hiding this comment

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

Please see my comment about the pod version.

Otherwise, LGTM.

@jaclync
Copy link
Contributor Author

jaclync commented Jan 15, 2021

Thank you for the prompt review, Stephenie! Merging this now 😄

@jaclync jaclync merged commit 8e91f82 into develop Jan 15, 2021
@jaclync jaclync deleted the wcios-2716/color-updates branch January 15, 2021 06:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Log In: some colors are off in dark mode

3 participants