-
Notifications
You must be signed in to change notification settings - Fork 805
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
Widgets: Hide Simple Payments create and edit buttons in widgets.php #9811
Widgets: Hide Simple Payments create and edit buttons in widgets.php #9811
Conversation
20f0b95
to
f9a214c
Compare
} ); | ||
|
||
function initWidget( widgetContainer ) { | ||
var widgetForm = widgetContainer.find( '> .widget-inside > .form, > .widget-inside > form' ); |
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.
No need for such a large refactor, but this may be a better fit for a delegated event handler, for example if we have a consistent parent such as:
$('.wpbody-content').on( 'click', '.jetpack-simple-payments-add-product', showAddNewForm );
This would allow us to only need to bind events once, but we'd need to rewrite handlers to be able to find the relevant widget to react to the correct form. Usually it's enough to do actions relative the to the closest parent SP widget.
} | ||
|
||
var productEmail = widgetForm.find( '.jetpack-simple-payments-form-product-email' ).val(); | ||
var isProductEmailValid = /^((([a-z]|\d|[!#\$%&'\*\+\-\/=\?\^_`{\|}~]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])+(\.([a-z]|\d|[!#\$%&'\*\+\-\/=\?\^_`{\|}~]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])+)*)|((\x22)((((\x20|\x09)*(\x0d\x0a))?(\x20|\x09)+)?(([\x01-\x08\x0b\x0c\x0e-\x1f\x7f]|\x21|[\x23-\x5b]|[\x5d-\x7e]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(\\([\x01-\x09\x0b\x0c\x0d-\x7f]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF]))))*(((\x20|\x09)*(\x0d\x0a))?(\x20|\x09)+)?(\x22)))@((([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.)+(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.?$/i.test( productEmail ); |
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.
This is likely a copy pasta from elsewhere, but where did we get this one from? As a warning getting email validation right in a regex is rather difficult (the official spec is quite complex) so oftentimes folks can only be certain if we email something and it works. So expect a few false positive and negatives with valid/invalid emails, even if this looks quite long.
@apeatling how do you feel about the compromise here on the https://helpingcat.wpsandbox.me/wp-admin/widgets.php page? Due to the approaching deadline, I think I'd be okay with this provided that we followed up on improving this flow in Jetpack release after. |
@gwwar Sorry, I think unrelated work going on in the feature branch messed up the diff of this PR. |
I'd be okay with going forward with this if we're running out of time for this release. Let's get another 👀 on copy before 🚢 , and write another issue to follow up or continue the convo on what we should do in this section. |
199c320
to
b710e5a
Compare
There's nothing wrong with the language you're proposing. Personally tho, I'm inclined to be more positive and direct: "This widget adds a payment button of your choice to your sidebar. To create or edit the payment buttons themselves, use the Customizer," where "use the Customizer" is a link. Tell them what to do and lose the "can only" -- close to what @gwwar's suggesting -- and offer a little explanation as to why this item is available in appearance > widgets at all if you can't create buttons there. |
@Copons with the latest changes on the feature branch, there's a duplication of messages when the user has no products: |
@rodrigoi Good catch! |
* Widgets: Adds support for Simple Payment Buttons as Widgets * Simple Payments Widget: Add style overrides (#9580) Override the media query and ensure that Simple Payments widgets are always displayed as a single column. * Widgets: Only render the Simple Payments widget if its button exists (#9673) In the frontend, only show the widget if the Simple Payments shortcode is parsed successfully. In the customizer, show the widget regardless, so that it can be modified via the pencil icon. * Simple Payment Widget: Manage Products from the Customizer (#9699) * Customizer: Simple Payments Widget breaks when starting without products (#9809) * Widgets: Hide Simple Payments create and edit buttons in widgets.php (#9811) * Widgets: Record events for the Simple Payments widget (#9803) * Widgets: Add Plan Check to the Simple Payments Widget (#9824) * Customizer: keep Simple Payments Widget Customizer in sync between instances (#9814) * Customizer: improves price validation on the Simple Payments Widget Customizer * Customizer: improves the behaviour of the action buttons on the Simple Payments Widget Customizer
Context: Automattic/wp-calypso#25576
The upcoming Simple Payments widget does not work in the Appearance/Widget page.
At a first look, it may require a heavy rework of its JS logic, that could probably make us miss the next JP release.
This is not exactly a fix, but a quick (and dirty) way to hide a feature from a section where it's broken, while keeping it in the section where it works, and all by remaining on time for the next JP release.
Changes proposed in this Pull Request:
Testing instructions:
wp-admin/customize.php
, open the Widgets panel, and add a Simple Payments widget.wp-admin/widgets.php
, and add a Simple Payments widget.