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

Customizer: Simple Payments Widget breaks when starting without products #9809

Conversation

rodrigoi
Copy link
Contributor

This PR fixes a number of issues with the Simple Payments Widget Customizer when the user has either a single product, or no product at all, including:

  • Showing a message when the user has no products.
  • Correctly loading the product information on the Edit form when there's a single product.
  • Correctly loading the product information on the Edit form after creating the first product.
  • Preserving the product drop down list selection after a Edit action when the list has more than one product.
  • Fixing some PHP warnings present on some themes.

Changes proposed in this Pull Request:

The customizer tool now shows a message when the user has no products loaded.

Before After
screen shot 2018-06-21 at 15 43 31 screen shot 2018-06-21 at 15 46 45

It also makes the first product the explicit default on $instance when the widget is first added, instead of having the widget function make the choice.

It also adds logic to customizer.js to manage the visibility of the new message and the product drop down list.

Testing instructions:

For all cases, please start with an Professional Jetpack site with no previously created Simple Payment products. If you have some, please delete them on Calypso's Editor before testing this PR.

PHP Warnings

Adding a widget when the user has no products fires this PHP warning (as seen on the error log):

PHP Notice:  Trying to get property of non-object in /var/www/html/wp-content/plugins/jetpack/modules/widgets/simple-payments.php on line 269
PHP Stack trace:
PHP   1. {main}() /var/www/html/index.php:0
PHP   2. require() /var/www/html/index.php:17
PHP   3. require_once() /var/www/html/wp-blog-header.php:19
PHP   4. include() /var/www/html/wp-includes/template-loader.php:74
PHP   5. get_footer() /var/www/html/wp-content/themes/twentyseventeen/index.php:71
PHP   6. locate_template() /var/www/html/wp-includes/general-template.php:76
PHP   7. load_template() /var/www/html/wp-includes/template.php:647
PHP   8. require_once() /var/www/html/wp-includes/template.php:688
PHP   9. get_template_part() /var/www/html/wp-content/themes/twentyseventeen/footer.php:22
PHP  10. locate_template() /var/www/html/wp-includes/general-template.php:155
PHP  11. load_template() /var/www/html/wp-includes/template.php:647
PHP  12. require() /var/www/html/wp-includes/template.php:690
PHP  13. dynamic_sidebar() /var/www/html/wp-content/themes/twentyseventeen/template-parts/footer/footer-widgets.php:23
PHP  14. WP_Widget->display_callback() /var/www/html/wp-includes/widgets.php:742
PHP  15. Jetpack_Simple_Payments_Widget->widget() /var/www/html/wp-includes/class-wp-widget.php:372

That may show on some themes:

screen shot 2018-06-21 at 15 56 34

To verify that this is no longer the case, just add a SP Widget to a sidebar/footer on a site with no previous products and verify that the warning is now present on the error_log or in the page itself.

Edit a Product after adding it
  • Add a SP Widget to a sidebar/footer on a site with no previously created products.
    • it should show the new message
  • Click Add New and create a new product by filling the form.
  • Click Edit Selected without interacting with any other element on the page.

The form should load with the correct information of the recently added product. The Customizer preview should update any changes live, and the changes should be persisted without problems.

Repeat for as many products as you'd like.

Edit a Single Product
  • Add a SP Widget to a sidebar/footer on a site with only one product
    • the only product should load on the customizer preview correctly.
  • Click Edit Selected without interacting with any other element on the page.

The form should load with the correct information of the single product. The Customizer preview should update any changes live, and the changes should be persisted without problems.

Delete all Products
  • Add a SP Widget to a sidebar/footer on a site with multiple products.
  • Click Edit Selected and then Delete. Confirm the action.
  • Repeat for all products on the drop down list.

When no more products are present, the empty list message should be displayed and the drop down list is no longer visible.

  • Add a new product by clicking Add New and filling the form.

The drop down list should be restored, the empty list message hidden and the new product should load correctly on the customizer preview.

  • Click Edit Selected

The form should load the newly created product, and changes should be correctly saved.

Known issues:

  • The Edit Selected button is not disabled when the user has no products
  • The Delete button is not disabled when the user is adding a new product.

@rodrigoi rodrigoi added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Extra Sidebar Widgets [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Pri] High Simple Payments [Status] Needs Copy Review Copy has been added. Marketing will be notified for a copy review. labels Jun 21, 2018
@rodrigoi rodrigoi added this to the 6.3 milestone Jun 21, 2018
@rodrigoi rodrigoi self-assigned this Jun 21, 2018
@rodrigoi rodrigoi requested a review from a team June 21, 2018 19:49
@rodrigoi rodrigoi requested a review from a team as a code owner June 21, 2018 19:49
@Copons
Copy link
Contributor

Copons commented Jun 22, 2018

I'd like to propose two changes relevant to this PR:

  • If there are no SP products, hide or disable the Edit Selected button, otherwise it's unclear what it's actually editing.

  • Add an empty option for the product selector, and use it as default for new SP widgets.
    I know that the widget requires a SP, but technically it doesn't, does it?
    If there is no SP selected, the widget simply outputs an empty content; imho that's fine from a UX standpoint, because it's obvious that if you don't select a SP, the widget won't show anything.
    This might also simplify the JS: we wouldn't need to listen to widget events anymore, but just to the select's on change.
    Also, we wouldn't force one arbitrarily selected SP to the user, but we'd implicitly guide them toward selecting an existing SP, or adding a new one.

@rodrigoi rodrigoi force-pushed the add/simple-payments-widget branch 2 times, most recently from 20f0b95 to f9a214c Compare June 22, 2018 18:41
@gwwar
Copy link
Contributor

gwwar commented Jun 22, 2018

🤔 Would the following be possible? @Copons @rodrigoi

Add a top default dropdown option, which contextually changes the button text to "create"
screen shot 2018-06-22 at 3 45 06 pm

If we select an existing button, text changes to "edit"
screen shot 2018-06-22 at 3 46 34 pm

Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

I agree that we can improve the UX further as @Copons suggests, but let's add to that in follow up PRs. I did some light manual testing, and it looks like this fixes a number of issues. Let's resolve conflicts and land this in the feature branch. Thanks @rodrigoi !

@Copons
Copy link
Contributor

Copons commented Jun 23, 2018

Add a top default dropdown option, which contextually changes the button text to "create"

@gwwar Technically yes, but folks might get confused into thinking that the only choice they have is to create a new SP (good if they actually don't have any SP yet; bad otherwise).
Might be because in your screen the select is greyed out, but still, I don't know.

Though, I agree that this is too big for this PR, and should move into a follow up.

But! I still stand behind my first point, which should be simple enough to implement here.
(cc @rodrigoi: feel free to disprove me here, I'm not as familiar as you with this codebase)

If there are no SP products, hide or disable the Edit Selected button, otherwise it's unclear what it's actually editing.

@rodrigoi rodrigoi force-pushed the add/simple-payments-widget-empty-product-list branch from 0a097ca to 573f505 Compare June 23, 2018 16:23
@rodrigoi rodrigoi merged commit ad2ad4c into add/simple-payments-widget Jun 23, 2018
@rodrigoi rodrigoi deleted the add/simple-payments-widget-empty-product-list branch June 23, 2018 16:26
oskosk pushed a commit that referenced this pull request Jun 26, 2018
* 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
@kraftbj kraftbj removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Sep 29, 2021
@jeherve jeherve added the [Feature] Pay with PayPal aka Simple Payments label Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Extra Sidebar Widgets [Feature] Pay with PayPal aka Simple Payments [Pri] High [Status] Needs Copy Review Copy has been added. Marketing will be notified for a copy review. Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants