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

D10 - Replace jquery.once with new once API #773

Merged
merged 2 commits into from
Aug 14, 2022

Conversation

jitendrapurohit
Copy link
Collaborator

@jitendrapurohit jitendrapurohit commented Aug 7, 2022

Overview

https://www.drupal.org/node/3158256

Before

Deprecated usage of jquery.once. CiviCRM settings page does not load -

webform_civicrm_error

After

Replaced with core/once. CiviCRM settings page loads fine -

image

Technical Details

Follows the similar approach done in core webform module - https://www.drupal.org/project/webform/issues/3295609

Comments

@KarinG @demeritcowboy

@KarinG
Copy link
Collaborator

KarinG commented Aug 7, 2022

cc: @demeritcowboy - follow up PR re: D10 readiness!

I think we (only) have one test failure ->

There was 1 error:

  1. Drupal\Tests\webform_civicrm\FunctionalJavascript\ContributionDummyTest::testCurrentEmployer
    Behat\Mink\Exception\ElementNotFoundException: Select option with value|text "Default Organization" not found.

@KarinG
Copy link
Collaborator

KarinG commented Aug 9, 2022

Hi @jitendrapurohit - do you know why the order of the arguments changed?

e.g.

var wfCiviContact = (function ($, D) {
var wfCiviContact = (function (D, $, once) {

@jitendrapurohit
Copy link
Collaborator Author

@KarinG Ah, I just followed the before/after code changes from https://www.drupal.org/node/3158256, but don't really think it matters since its just used to wrap the function in a closure.

@KarinG
Copy link
Collaborator

KarinG commented Aug 14, 2022

Ah ok I see. I've pushed a trivial commit to re-run the tests.

@KarinG
Copy link
Collaborator

KarinG commented Aug 14, 2022

All green - merging 🎉

@KarinG KarinG merged commit 361b1bc into colemanw:6.x Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants