Problem/Motivation

Setup picker is executed multiple times in drupal behaviours which can cause picker markup to be broken.

Proposed resolution

Wrap the setup picker code in once condition so that markup is not duplicated.

CommentFileSizeAuthor
#12 interdiff-10-12.txt554 bytesnashkrammer
#12 setup-picker-once-3319692-12.patch738 bytesnashkrammer
#10 interdiff-8-10.txt331 bytesnashkrammer
#10 setup-picker-once-3319692-10.patch743 bytesnashkrammer
#8 setup-picker-once-3319692-8.patch966 bytessaurabhpandit26
#7 setup-picker-once-3319692-7.patch771 bytessaurabhpandit26
#6 setup-picker-once-3319692-5.patch735 bytessaurabhpandit26
#2 setup-picker-once-3319692-1.patch759 bytessaurabhpandit26

Comments

saurabhpandit26 created an issue. See original summary.

saurabhpandit26’s picture

saurabhpandit26 commented
StatusFileSize
new setup-picker-once-3319692-1.patch759 bytes
nikunjkotecha’s picture

nikunjkotecha
he/him
Primary language Gujarati
Location India, Gujarat, Rajkot
commented

@Saurabh please raise MR so it is easy to review

+ stlib_picker.setupPicker(jQuery('#myPicker', context), drupalSettings.sharethis.service_string_markup, drupal_st.serviceCallback);

Here we can use "jQuery(this)" instead of jQuery('#myPicker', context) as it is already inside .once

nikunjkotecha’s picture

nikunjkotecha
he/him
Primary language Gujarati
Location India, Gujarat, Rajkot
commented
Status: Active » Needs work
saurabhpandit26’s picture

saurabhpandit26 commented

@Nikunj unable to create an issue fork for this, so uploading a new patch again.

saurabhpandit26’s picture

saurabhpandit26 commented
StatusFileSize
new setup-picker-once-3319692-5.patch735 bytes
saurabhpandit26’s picture

saurabhpandit26 commented
StatusFileSize
new setup-picker-once-3319692-7.patch771 bytes

Was getting undefined function error in patch #5

saurabhpandit26’s picture

saurabhpandit26 commented
StatusFileSize
new setup-picker-once-3319692-8.patch966 bytes

Refactored patch again to use jquery this object.

vivek panicker’s picture

vivek panicker commented

Comments regarding the patch #8

diff --git a/js/sharethis.form.js b/js/sharethis.form.js
index 8b15b21..87efe49 100644
--- a/js/sharethis.form.js
+++ b/js/sharethis.form.js
@@ -3,7 +3,7 @@
  * This file contains most of the code for the configuration page.
  */
 
-(function ($, window, drupalSettings, stlib_picker) {
+ (function ($, window, drupalSettings, stlib_picker) {
 
   'use strict';
 
@@ -157,9 +157,11 @@
 
   Drupal.behaviors.shareThisForm = {
     attach: function (context) {
-      stlib_picker.setupPicker(jQuery('#myPicker', context), drupalSettings.sharethis.service_string_markup, drupal_st.serviceCallback);
-      drupal_st.addEvents();
-      drupal_st.setupServiceText();
+      jQuery('#myPicker', context).once('setup_picker_once').each(function () {
+        stlib_picker.setupPicker(jQuery(this), drupalSettings.sharethis.service_string_markup, drupal_st.serviceCallback);
+        drupal_st.addEvents();
+        drupal_st.setupServiceText();
+      });
     }
   };

1/ There seems to be an extra space before (function ($`.
2/ jQuery('#myPicker', context) can be jQuery('#myPicker', context)

nashkrammer’s picture

nashkrammer commented
StatusFileSize
new setup-picker-once-3319692-10.patch743 bytes
new interdiff-8-10.txt331 bytes

1/ There seems to be an extra space before (function ($`. Fixed
2/ jQuery('#myPicker', context) can be jQuery('#myPicker', context) any particular reason to have indent here.

vivek panicker’s picture

vivek panicker commented

Sorry, my bad.
Copy + paste error.
What I meant was that can we use `$` instead of `jQuery` there?
jQuery('#myPicker', context) can be $('#myPicker', context)

nashkrammer’s picture

nashkrammer commented
StatusFileSize
new setup-picker-once-3319692-12.patch738 bytes
new interdiff-10-12.txt554 bytes

Fixed https://www.drupal.org/project/sharethis/issues/3319692#comment-14813634