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

63: filter data, rearrange plugin includes #81

Merged
merged 12 commits into from
Nov 6, 2018
Merged

63: filter data, rearrange plugin includes #81

merged 12 commits into from
Nov 6, 2018

Conversation

benlk
Copy link
Collaborator

@benlk benlk commented Oct 24, 2018

This PR is a WIP. 🚨

Changes

Housekeeping:

  • rearranges plugin files so that each of the named classes has its own file, rather than having multiple classes per file: to help developers keep things straight

Implementation of #63 (comment) to allow filtering of options used by jquery.dfp.js:

  • make a filter on $data before it's localised by calling apply_filters( 'dfw_js_data', $data ) on it
  • figure out how to get jquery.dfw.js to load arbitrary options from the dfw option, same as the footer script.
    • What would probably work here is getting the keys on $data to be the appropriate keys for use as the configuration options that jQuery.dfp is looking for. But this might break other uses of network_code, and we should keep that in the dfw JSON object for legacy support.
  • refactor DoubleClick::footer_script to draw from window.dfw instead of its own options

Why

For #63 and GDPR compliance

Testing/Questions

Questions that need to be answered before merging:

  • does it work?

Steps to test this PR:

  1. ...

@benlk benlk added this to the 0.3 milestone Oct 24, 2018
@benlk
Copy link
Collaborator Author

benlk commented Oct 31, 2018

This is probably not going to make it into 0.3 because of time constraints, but there's room for it in a follow-up release.

@benlk
Copy link
Collaborator Author

benlk commented Nov 2, 2018

refactoring in progress, but the network code is not being output! dfpId is not being set to the value of the network_code option in window.dfp_options. In js/jquery.dfw.js, Object.assign should be returning the contents of dfw with the specified items overwriting the values in window.dfw.

@benlk benlk modified the milestones: 0.3, 0.4 Nov 5, 2018
@benlk
Copy link
Collaborator Author

benlk commented Nov 5, 2018

Bumping to 0.4, sorry.

@benlk
Copy link
Collaborator Author

benlk commented Nov 6, 2018

Testing complete locally; lazy-loaded and non- alike render.

Testing complete on a staging site using DFW to serve ads:

  • ads still render
  • UI has new labels

@benlk benlk merged commit bf4fcb5 into master Nov 6, 2018
@benlk benlk changed the title 63 filter data 63: filter data, rearrange plugin includes Nov 6, 2018
@benlk benlk modified the milestones: 0.4, 0.3 Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant