Product SKU Generator: Release #28
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t occurs with Square for WooCommerce When the [Square for WooCommerce](https://woocommerce.com/products/square/) plugin is activated, the following JavaScrip error occurs on the the product edit page because the SKU field is removed: wc-square-admin-products.min.js:188 Uncaught TypeError: Cannot read property 'trim' of undefined This commit replaces the inline SKU with a disabled SKU input to address conflicts like these.
wvega
requested changes
Nov 23, 2020
There was a problem hiding this comment.
@tamarazuk thank you for putting this fix together! QA passes for me and changes look good.
All I have a is a suggestion to split is_minmum_wc_version_met() to keep the method small and with a single responsibility.
Comment on lines
+247
to
+250
| if ( is_admin() && ! is_ajax() && ! has_action( 'admin_notices', [ $this, 'render_outdated_wc_version_notice' ] ) ) { | ||
|
|
||
| add_action( 'admin_notices', [ $this, 'render_outdated_wc_version_notice' ] ); | ||
| } |
There was a problem hiding this comment.
Can we extract the nested if statement into another method (perhaps maybe_add_admin_notices_hooks())?
I think right now the method has two responsibilities: determining whether the version is met and registering the hook handler that will render the admin notice. To keep the methods simpler, we could extract those lines into a separate method and call it before halting functionality.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses a conflict that occurs with Square for WooCommerce and swaps the WooCommerce version check to use the
WC_VERSIONconstantStories
QA
confirm that SKUs display as expected, now in a disabled input
confirm that SKUs generate as expected
confirm that the plugin doesn't load and a notice appears if
WC_VERSIONconstant returns a value less than3.0.9(you can do so editing this line)