Problem/Motivation

The module does not use a lot of JS, but still uses jQuery which should not be required. Instead replace it by plain JS.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork ad-3534874

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

anybody created an issue. See original summary.

lrwebks made their first commit to this issue’s fork.

lrwebks opened merge request !26

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: lrwebks » Unassigned

@lrwebks thinks it's too heavy for him :)

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » lrwebks
Status: Active » Needs review

@lrwebks it's not that hard as you think, please review and test manually by time.

Guess it's a great chance to learn how it works (and be less in fear then for the future :))

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

PS: My implementation is untested, so please review and try carefully!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@grevil: Please review code-wise.
@lrwebks please test if this works as expected and nothing is broken

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
lrwebks’s picture

lrwebks commented

@anybody: Tested it with multiple different ad types and it works as expected! Even the problem I had personally, where the ads would only display for regular users and not admins, seems to have resolved itself. That leads me to believe it might actually have been a JQuery+Drupal quirk as well… Who knows?

lrwebks’s picture

lrwebks commented

Re #4 by the way: The solution you have implemented is totally understandable to me and I can definitely grasp the fetch API. But converting that myself would have certainly wasted more worker hours than it should! So, glad that someone else took care of it.

lrwebks’s picture

lrwebks commented
Status: Needs review » Reviewed & tested by the community

Code-wise, this also looks good to me!

  • lrwebks committed f6df46dc on 11.x 
    Issue #3534874 by anybody, lrwebks: Replace jQuery by vanilla JS
lrwebks’s picture

lrwebks commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.