artsy / emission

⚠️ Deprecated repo, moved to artsy/eigen ➡️ React Native Components
http://artsy.github.io/blog/2018/04/17/making-a-components-pod/
MIT License
618 stars 78 forks source link

PURCHASE-1584: Update AuctionCountDownTimer to handle liveStartAt #1912

Closed lilyfromseattle closed 5 years ago

lilyfromseattle commented 5 years ago

PURCHASE-1584

The AuctionCountDownTimer component currently does not handle artworks with auction liveStartAt timestamps.

The component should also be able to handle a single date even if another isn't available. E.g. an artwork with a startAt but not an endAt date, or vice versa.

Instead of relying on formattedStartDateTime I made a method that returns the strings based on auctionState, so that it will update when the state changes. I also added some date formatting helper functions.

Screen Shot 2019-10-09 at 5 39 29 PM

skip_new_tests

kierangillen commented 5 years ago

@sweir27 @lilyfromseattle I think it could be really helpful to write all the states that are possible clearly in a comment in this file, and then from there work out the logic for each condition. Then we can use this as a reference for tests and for all the displays. It could also be helpful then to capture a specific state and have all of the functions use that as a source of truth, that way the coundtowntimer and the date display are using the same source and we will never see weird state conditions.

sweir27 commented 5 years ago

@kierangillen good idea on documenting.

@lilyfromseattle would you like to take a stab? I'm happy to give feedback!

Re: the question of whether the countdown timer and date display should depend on auction state-- they could but when we discussed before it didn't seem necessary for it to update in realtime. If it feels like a clearer architectural choice we should definitely do that, but will leave it up to you two.

kierangillen commented 5 years ago

The parent component has a setInterval keeping track of the auction state, we should still use that as an overall source of truth and that will update this component when the auction state changes. We should probably look into having some local state also in this component for all of the different display possibilities.

lilyfromseattle commented 5 years ago

@kierangillen Not sure what you mean, as I'm already using auctionState as a source of truth