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

Adds default 'bid' button label behaviour. #2667

Merged
merged 1 commit into from Jul 16, 2018

Conversation

Projects
None yet
3 participants
@ashfurrow
Member

ashfurrow commented Jul 16, 2018

This is a fix for Purchase-240. The main changes are to the code in ARBidButton.m. I've added a new, more specific method for setting the auction state on the bid button that let's us define if the button should said "Bid" by default, or "Register". "Register" was the old default value, so I used ARBidButtonIntentRegister as a default value for the more specific function. This is a common pattern in Objective-C, but it can be confusing at first, so let me know if I can clarify.

The "bid button" class would more accurately be described as an "auction button" because it contains centralized logic for the CTA we show the user at any point in a sale. It could say "Bid" or "Register" or "Registration Pending" or "Registration Closed", etc. Complicating things, this button gets used in the following contexts, so any change we make needs to be backwards-compatible.

  • AuctionViewController (at the top of the list of lots)
  • ARArtworkActionsView (presented in an ArtworkView)
  • AuctionInformationViewController (accessible via AuctionViewController as well as LAI)

I added a new snapshot test for the non-default intent of bidding, as well as updated the existing snapshot for tests that contain this button.

Continues work from #2659

@ashfurrow ashfurrow requested a review from erikdstock Jul 16, 2018

@ashfurrow ashfurrow force-pushed the ashfurrow-default-bid-button branch from 004da80 to fd1a968 Jul 16, 2018

@erikdstock

Looks good! Thanks!

@sweir27

This comment has been minimized.

Collaborator

sweir27 commented Jul 16, 2018

Ah, cool!

@sweir27 sweir27 merged commit e6f4db5 into master Jul 16, 2018

2 checks passed

ci/circleci: build-and-test Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment