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

Hide Payment Request container until mounting Element #959

Merged
merged 2 commits into from Sep 4, 2019

Conversation

dechov
Copy link
Contributor

@dechov dechov commented Aug 14, 2019

Fixes #855

Changes proposed in this Pull Request:

Hide Payment Request container until the Element is actually being mounted there. This affects the loading state but primarily it improves cases where no button would show.

Testing in Firefox:

master this branch

I also verified that the appearance is unchanged when the button does show (testing with jurassic.ninja).


  • Make sure your changes respect WordPress' coding standards.
  • Did you make changes, or create a new .js file? If Gruntfile.js exists in the repo, make sure to run grunt.

@@ -406,12 +406,9 @@ jQuery( function( $ ) {
}

if ( $( '#wc-stripe-payment-request-button' ).length ) {
$( '#wc-stripe-payment-request-wrapper, #wc-stripe-payment-request-button-separator' ).show();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you move the show() call to before the mount() call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if it makes a difference but I'd felt there was a slim chance that the mounting of the button could suffer from the container being hidden (if it reads any layout measurements, for instance), and it made sense to me that the separator would also just be shown earlier as well. Didn't actually test it the other way, but that was why – do you feel differently / prefer to keep one or both elements hidden until after mounting?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was just curious if this change was intentional, since before this mount() was before show() and seemed to be working well. Your explanation makes sense, and worst-case-scenario, it just makes no difference :)

Copy link
Contributor

@DanReyLop DanReyLop left a comment

Choose a reason for hiding this comment

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

🚢 it

@dechov dechov merged commit db46121 into master Sep 4, 2019
@dechov dechov deleted the fix/855-hide-payment-request-container-until-mount branch September 4, 2019 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enabling Payment Request button changes position of Add to cart button
2 participants