-
Notifications
You must be signed in to change notification settings - Fork 612
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
PAYMENTS-1160: Ensure Apple Pay button is hidden for incompatible browsers. #1084
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems legit but you've made Travis unhappy
It seems you have to apply |
@philipmuir may be able to help you test it locally if you need (and then you can both teach us!) |
Thanks to Travis' prompting, I decided to take a different approach to this, and instead remove the |
Dismissing due to changed implementation.
@PascalZajac can you add a changelog entry before you merge this one. |
Sure thing, under the Draft heading right? |
Yes it should be in the draft section. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks @PascalZajac for making the change, LGTM!
What?
Remove an unnecessary
display: block
rule that was causing the Apple Pay button to appear in the Cart Modal even for browsers that do not support Apple Pay.Why?
Shopper confusion. The rule in question is ignoring the
apple-pay-supported
class that gets attached to thebody
element, forcing the button to appear in all browsers. My first solution was to fix this by changing the nesting of the rules to respect this class, but then I realised that thedisplay: block
rule itself is unnecessary, because it will be inherited from the rule defined on L33.Screenshots (if appropriate)
Since I can't enable Apple Pay for local development, I have screenshots showing the HTML structure in Chrome and Safari respectively to prove that this should work.
![screen shot 2017-09-04 at 3 53 42 pm](https://user-images.githubusercontent.com/813373/30038434-1b8b3862-918a-11e7-83b8-02e83441d6ca.png)
![screen shot 2017-09-04 at 3 53 21 pm](https://user-images.githubusercontent.com/813373/30038404-c199f15e-9189-11e7-9c19-630c1f06c464.png)
Ping @bigcommerce/stencil-team @bigcommerce/payments