Fix Customizer failing after upgrade; fix widget settings #477
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
Contributor
joshcanhelp
commented
Jun 5, 2018
•
joshcanhelp
commented
- Customizer was failing without Bootstrap JS dependency being loaded (Javascript error loading Customize #476)
- Refactored admin scripts and styles to avoid dependency issues in the future
- Fix Social Amplification widget error when saving in the Customizer
- Fix "Button Text" field showing for embedded login widget
- Fix Icon URL not live updating for embedded login widget
- Removed unused Facebook message field for Social Amplification widget
- Change settings page field descriptions for social app keys/secrets
- Add missing translation in wp-admin
joshcanhelp
changed the title
joshcanhelp
force-pushed
the
fix-widgets-in-customizer
branch
from
d8e0d9a
to
b399a36
Compare
joshcanhelp
commented
Jun 5, 2018
| @@ -31,7 +31,7 @@ jQuery(document).ready(function($) { | |||
| // Set the frame callback | |||
| media_frame.on('select', function() { | |||
| var attachment = media_frame.state().get('selection').first().toJSON(); | |||
| $('#'+related_control_id).val(attachment.url); | |||
| $('#'+related_control_id).val(attachment.url).change(); | |||
There was a problem hiding this comment.
Triggers a refresh in the Preview pane
joshcanhelp
commented
Jun 5, 2018
| @@ -15,26 +15,21 @@ protected function getWidgetId() { | |||
| } | |||
|
|
|||
| protected function getWidgetName() { | |||
| return 'Auth0 Lock Embed'; | |||
| return __( 'Auth0 Login', 'wp-auth0' ); | |||
There was a problem hiding this comment.
Missing translation
joshcanhelp
commented
Jun 5, 2018
| wp_enqueue_media(); | ||
| wp_enqueue_script( 'wpa0_admin', WPA0_PLUGIN_JS_URL . 'admin.js', array( 'jquery' ), WPA0_VERSION ); |
There was a problem hiding this comment.
Moving script data to wp_register_script() call
joshcanhelp
commented
Jun 5, 2018
| @@ -171,8 +171,6 @@ protected function defaults() { | |||
| 'last_step' => 1, | |||
| 'migration_token_id' => null, | |||
| 'jwt_auth_integration' => false, | |||
| 'amplificator_title' => '', | |||
joshcanhelp
commented
Jun 5, 2018
| public function form( $instance ) { | ||
| require WPA0_PLUGIN_DIR . 'templates/a0-widget-amplificator.php'; | ||
| $fields = array( |
There was a problem hiding this comment.
Replacing what was removed in templates/a0-widget-amplificator.php below
joshcanhelp
commented
Jun 5, 2018
| $options->set( 'social_twitter_message', $new_instance['social_twitter_message'] ); | ||
| $options->set( 'amplificator_title', $new_instance['amplificator_title'] ); | ||
| $options->set( 'amplificator_subtitle', $new_instance['amplificator_subtitle'] ); | ||
| $new_instance['social_twitter_message'] = sanitize_text_field( $new_instance['social_twitter_message'] ); |
There was a problem hiding this comment.
Need to set this for the widget instance, not in a global setting (WordPress Customizer stops too many options from being set ... probably a security measure?)
joshcanhelp
commented
Jun 5, 2018
| } | ||
|
|
||
| public function update( $new_instance, $old_instance ) { | ||
| $options = WP_Auth0_Options::Instance(); | ||
| $options->set( 'social_facebook_message', $new_instance['social_facebook_message'] ); |
There was a problem hiding this comment.
Not used in the widget, removed.
joshcanhelp
commented
Jun 5, 2018
|
|
||
| if ( !empty( $amplificator_title ) ) { | ||
| echo "<h2 class=\"widget-title\">$amplificator_title</h2>"; | ||
| if ( ! empty( $instance['amplificator_title'] ) ) { |
There was a problem hiding this comment.
Switching from global setting to instance and adding output sanitization
joshcanhelp
commented
Jun 5, 2018
| $js_function = "javascript: void window.open('https://twitter.com/share?url=$current_page_url&text=$content', '', 'height=300, width=600');"; | ||
| break; | ||
|
|
||
| case 'facebook': |
There was a problem hiding this comment.
Refactoring for clarity
joshcanhelp
commented
Jun 5, 2018
| @@ -209,6 +209,25 @@ public function init() { | |||
| * Enqueue scripts for all Auth0 wp-admin pages | |||
| */ | |||
| public function admin_enqueue() { | |||
| // Register admin styles | |||
There was a problem hiding this comment.
Registering these styles and scripts here lets use enqueue at any point down the line. Also setting correct dependencies to avoid the bug we're fixing in the future.
lbalmaceda
approved these changes
Jun 6, 2018
| @@ -595,6 +596,8 @@ public function render_social_twitter_secret( $args = array() ) { | |||
| public function render_social_facebook_key( $args = array() ) { | |||
| $this->render_social_key_field( $args['label_for'], $args['opt_name'] ); | |||
| $this->render_field_description( | |||
| __( 'Facebook app key for the Social Amplification Widget. ', 'wp-auth0' ) . | |||
| __( 'The app used here needs to have "publish_actions" permission. ', 'wp-auth0' ) . | |||
There was a problem hiding this comment.
Suggestion, if you think the links will last, link to facebook permissions' docs ?
| @@ -555,7 +555,8 @@ public function render_custom_signup_fields( $args = array() ) { | |||
| public function render_social_twitter_key( $args = array() ) { | |||
| $this->render_social_key_field( $args['label_for'], $args['opt_name'] ); | |||
| $this->render_field_description( | |||
| __( 'Used for the Social Amplification Widget. ', 'wp-auth0' ) . | |||
| __( 'Twitter app key for the Social Amplification Widget. ', 'wp-auth0' ) . | |||
| __( 'The app used here needs to have "read" and "write" permissions. ', 'wp-auth0' ) . | |||
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.