Fix Ad Blocker Warning Message based on modules connected state. #4065

kuasha420 · 2021-09-16T19:54:38Z

Summary

Addresses issue #3208

Relevant technical choices

Checklist

My code is tested and passes existing unit tests.
My code has an appropriate set of unit tests which all pass.
My code is backward-compatible with WordPress 4.7 and PHP 5.6.
My code follows the WordPress coding standards.
My code has proper inline documentation.
I have added a QA Brief on the issue linked above.
I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

github-actions · 2021-09-16T20:05:28Z

Size Change: +12.4 kB (+1%)

Total Size: 1.15 MB

Filename	Size	Change
`./dist/assets/css/admin.css`	0 B	-38.5 kB (removed)	🏆
`./dist/assets/css/adminbar.css`	0 B	-8 kB (removed)	🏆
`./dist/assets/css/wpdashboard.css`	0 B	-4.58 kB (removed)	🏆
`./dist/assets/js/33.********************.js`	3.12 kB	+1 B (0%)
`./dist/assets/js/googlesitekit-activation.********************.js`	38.6 kB	+727 B (+2%)
`./dist/assets/js/googlesitekit-adminbar.********************.js`	31.4 kB	+634 B (+2%)
`./dist/assets/js/googlesitekit-api.********************.js`	9.03 kB	-5 B (0%)
`./dist/assets/js/googlesitekit-base.********************.js`	1.59 kB	+1 B (0%)
`./dist/assets/js/googlesitekit-dashboard-details.********************.js`	45.6 kB	+1.24 kB (+3%)
`./dist/assets/js/googlesitekit-dashboard-splash.********************.js`	52.8 kB	+758 B (+1%)
`./dist/assets/js/googlesitekit-dashboard.********************.js`	45.8 kB	+1.22 kB (+3%)
`./dist/assets/js/googlesitekit-datastore-forms.********************.js`	8.74 kB	+11 B (0%)
`./dist/assets/js/googlesitekit-datastore-location.********************.js`	2.02 kB	-3 B (0%)
`./dist/assets/js/googlesitekit-datastore-site.********************.js`	13.2 kB	-10 B (0%)
`./dist/assets/js/googlesitekit-datastore-ui.********************.js`	8.7 kB	-9 B (0%)
`./dist/assets/js/googlesitekit-datastore-user.********************.js`	20.9 kB	-34 B (0%)
`./dist/assets/js/googlesitekit-idea-hub-notice.********************.js`	2.4 kB	-1 B (0%)
`./dist/assets/js/googlesitekit-idea-hub-post-list.********************.js`	23.8 kB	+216 B (+1%)
`./dist/assets/js/googlesitekit-module.********************.js`	45.1 kB	+760 B (+2%)
`./dist/assets/js/googlesitekit-modules-adsense.********************.js`	45.2 kB	+400 B (+1%)
`./dist/assets/js/googlesitekit-modules-analytics-4.********************.js`	18.8 kB	+4 B (0%)
`./dist/assets/js/googlesitekit-modules-analytics.********************.js`	66.9 kB	+1.14 kB (+2%)
`./dist/assets/js/googlesitekit-modules-idea-hub.********************.js`	24.6 kB	+816 B (+3%)
`./dist/assets/js/googlesitekit-modules-optimize.********************.js`	18.3 kB	-9 B (0%)
`./dist/assets/js/googlesitekit-modules-pagespeed-insights.********************.js`	16.8 kB	+457 B (+3%)
`./dist/assets/js/googlesitekit-modules-search-console.********************.js`	28.8 kB	+351 B (+1%)
`./dist/assets/js/googlesitekit-modules-subscribe-with-google.********************.js`	16.4 kB	-17 B (0%)
`./dist/assets/js/googlesitekit-modules-tagmanager.********************.js`	29.8 kB	-10 B (0%)
`./dist/assets/js/googlesitekit-settings.********************.js`	49.7 kB	+954 B (+2%)
`./dist/assets/js/googlesitekit-user-input.********************.js`	45.7 kB	+732 B (+2%)
`./dist/assets/js/googlesitekit-vendor.********************.js`	312 kB	+169 B (0%)
`./dist/assets/js/googlesitekit-widgets.********************.js`	12.5 kB	+1.11 kB (+10%)	⚠️
`./dist/assets/js/googlesitekit-wp-dashboard.********************.js`	33.5 kB	+790 B (+2%)
`./dist/assets/js/runtime.********************.js`	1.19 kB	+2 B (0%)
`./dist/assets/css/googlesitekit-admin-css.********************.css`	38.6 kB	+38.6 kB (new file)	🆕
`./dist/assets/css/googlesitekit-adminbar-css.********************.css`	8 kB	+8 kB (new file)	🆕
`./dist/assets/css/googlesitekit-wp-dashboard-css.********************.css`	4.58 kB	+4.58 kB (new file)	🆕

ℹ️ View Unchanged

Filename	Size
`./dist/assets/js/analytics-advanced-tracking.js`	769 B
`./dist/assets/js/googlesitekit-data.********************.js`	1.65 kB
`./dist/assets/js/googlesitekit-i18n.js`	3.92 kB
`./dist/assets/js/googlesitekit-modules.********************.js`	16.5 kB
`./dist/assets/js/googlesitekit-polyfills.********************.js`	378 B

_{compressed-size-action}

tofumatt

This needs some changes because it doesn't actually behave as-intended—see the issue I mentioned with the resolveSelect not working anymore. The E2E and Visual Regression tests are failing as a result.

I'll add a commit that should fix it though.

assets/js/modules/adsense/index.js

assets/js/modules/adsense/datastore/adblocker.js

tofumatt · 2021-09-22T21:55:17Z

assets/js/modules/adsense/index.js

 				.__experimentalResolveSelect( MODULES_ADSENSE )
-				.isAdBlockerActive();
+				.getAdBlockerWarningMessage();


This is the reason VRT and E2E tests were failing; resolveSelect will return a value after the selector's resolver has completed, but because the IB suggested changing the selector used, this returns immediately because getAdBlockerWarningMessage doesn't have an associated selector.

There are two options here:

continue using isAdBlockerActive here but getting the message later on if isAdBlockerActive is true

add a (redundant) resolver to getAdBlockerWarningMessage

I added a commit that does the former. It's not 100% in line with the IB but still fulfills the ACs and behaves as-intended.

google-cla · 2021-09-22T21:57:26Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

tofumatt · 2021-09-22T21:58:19Z

@googlebot I consent.

tofumatt

The rest of this works for me; I think there are a few string changes we should make but I'll leave it for the reviewer to confirm.

I'm moving this to merge review to get another look at the approach though.

google-cla · 2021-09-29T08:20:56Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

aaemnnosttv · 2021-09-29T08:21:46Z

@googlebot I consent.

aaemnnosttv · 2021-09-29T08:22:45Z

I pushed one commit to fix a doc block but otherwise this LGTM 👍

I like the changes to the wording as well but I'll leave this for @felixarntz to sign off on.

Co-authored-by: Matthew Riley MacPherson <hi@tofumatt.com>

Fix Ad Blocker Warning Message based on modules connected state.

823fa5a

google-cla bot added the cla: yes label Sep 16, 2021

kuasha420 added 4 commits September 18, 2021 01:09

Differenciate between adblocker detection state.

43d050c

Add test for getAdBlockerWarningMessage selector.

50de0f9

Remove log statements.

5e9c4e6

Update tests AdBlockerWarning component.

8e5134b

tofumatt requested changes Sep 22, 2021

View reviewed changes

Use a selector with a resolver for the resolveSelect call.

c2adf82

google-cla bot added cla: no and removed cla: yes labels Sep 22, 2021

google-cla bot added cla: yes and removed cla: no labels Sep 22, 2021

tofumatt approved these changes Sep 22, 2021

View reviewed changes

Update doc block for selector return.

8aaeb34

google-cla bot added cla: no and removed cla: yes labels Sep 29, 2021

google-cla bot added cla: yes and removed cla: no labels Sep 29, 2021

felixarntz and others added 2 commits September 29, 2021 10:06

Improve minor wording issues in ad blocker warning.

7aa2fda

Co-authored-by: Matthew Riley MacPherson <hi@tofumatt.com>

Update ad blocker wording accordingly in tests as well.

f09451c

felixarntz approved these changes Sep 29, 2021

View reviewed changes

felixarntz merged commit 165b2b2 into develop Sep 29, 2021

felixarntz deleted the enhancement/3208-adblocker-warning-msg branch September 29, 2021 17:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Ad Blocker Warning Message based on modules connected state. #4065

Fix Ad Blocker Warning Message based on modules connected state. #4065

kuasha420 commented Sep 16, 2021 •

edited

Loading

github-actions bot commented Sep 16, 2021 •

edited

Loading

tofumatt left a comment

tofumatt Sep 22, 2021

google-cla bot commented Sep 22, 2021

tofumatt commented Sep 22, 2021

tofumatt left a comment

google-cla bot commented Sep 29, 2021

aaemnnosttv commented Sep 29, 2021

aaemnnosttv commented Sep 29, 2021

Fix Ad Blocker Warning Message based on modules connected state. #4065

Fix Ad Blocker Warning Message based on modules connected state. #4065

Conversation

kuasha420 commented Sep 16, 2021 • edited Loading

Summary

Relevant technical choices

Checklist

github-actions bot commented Sep 16, 2021 • edited Loading

tofumatt left a comment

Choose a reason for hiding this comment

tofumatt Sep 22, 2021

Choose a reason for hiding this comment

google-cla bot commented Sep 22, 2021

tofumatt commented Sep 22, 2021

tofumatt left a comment

Choose a reason for hiding this comment

google-cla bot commented Sep 29, 2021

aaemnnosttv commented Sep 29, 2021

aaemnnosttv commented Sep 29, 2021

kuasha420 commented Sep 16, 2021 •

edited

Loading

github-actions bot commented Sep 16, 2021 •

edited

Loading