eclipse-thingweb / playground

Browser or Node.js based tool for validating and playing with W3C Thing Descriptions
http://plugfest.thingweb.io/playground/
Other
28 stars 22 forks source link

Implemented the analytics consent banner for Playground #581

Closed SergioCasCeb closed 2 months ago

SergioCasCeb commented 2 months ago

Google Analytics, as well as the respective banner for consent management, have been implemented on TD Playground following Eclipse's requirements, while also maintaining the same overall design as the consent banner from the Thingweb website."

netlify[bot] commented 2 months ago

Deploy Preview for thingweb-playground ready!

Name Link
Latest commit d13c6f3c3f4c6eb826cc770d903921b805c34144
Latest deploy log https://app.netlify.com/sites/thingweb-playground/deploys/6631c1c12f9a990008e53a69
Deploy Preview https://deploy-preview-581--thingweb-playground.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

Lighthouse
1 paths audited
Performance: 25 (🔴 down 27 from production)
Accessibility: 79 (no change from production)
Best Practices: 100 (no change from production)
SEO: 73 (no change from production)
PWA: 70 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

egekorkan commented 2 months ago

The eslint error is a bit weird to see since that is coming from google analytics. Otherwise, the integration looks good. When I head over to google analytics, I see TD Playground as page title in the realtime analytics and I see Eclipse Thingweb in the report snapshot. So they do not show up at the same time somehow?

Realtime: image

Reports Snapshot: image

Reading https://www.analyticsmania.com/post/subdomain-tracking-with-google-analytics-and-google-tag-manager/ means that we do not need multiple properties and with what I have observed, it is probably working to some extent. I have followed https://support.google.com/analytics/answer/10071811 and adding a condition and once this PR is merged, it should just work.

SergioCasCeb commented 2 months ago

I believe the eslint issue with the gtag function, is probably due to how Webpack handles the bundling,etc. Or it could also be that since the js bundle is not added until production, it cannot access the already instantiated gtag function in the HTML file.

egekorkan commented 2 months ago

The banner is causing the tests to fail since playwright cannot click on the many links and buttons under the banner. I think the footer should be clicked or hidden for the tests.

egekorkan commented 2 months ago

We can use snippet injection of netlify to add analytics: https://docs.netlify.com/site-deploys/post-processing/snippet-injection/ . But I leave it up to you @SergioCasCeb

SergioCasCeb commented 2 months ago

The banner is causing the tests to fail since playwright cannot click on the many links and buttons under the banner. I think the footer should be clicked or hidden for the tests.

Yeah, sorry about that. I forgot to also push the changes to the tests. I should be done in just a bit

SergioCasCeb commented 2 months ago

We can use snippet injection of netlify to add analytics: https://docs.netlify.com/site-deploys/post-processing/snippet-injection/ . But I leave it up to you @SergioCasCeb

Since the consent management, needs a really specific order to be executed, besides the fact that some of it depends on the local storage, I think it would be easier to just add it directly in the code.

Besides Playground is a one page application, so it doesn't really complicate anything, besides also allowing for testing.

egekorkan commented 2 months ago

After the recent changes, we now have 355 tests instead of 140 since the consent management tests are done for each. Can we optimize it by doing the consent management test once at the beginning of everything and then before each test, playwright just clicks deny and closes the banner? Otherwise the tests now pass

SergioCasCeb commented 2 months ago

After the recent changes, we now have 355 tests instead of 140 since the consent management tests are done for each. Can we optimize it by doing the consent management test once at the beginning of everything and then before each test, playwright just clicks deny and closes the banner? Otherwise the tests now pass

Just out of curiosity, where did the 355 number come from, for me it is only 142 tests, as well as in the visual testing pipeline.

I set it up exactly how you are explaining, before any test is done, the consent banner is checked for the default state, and the closed (so that is "one test"). Then there's an extra test to check that the update does occur when opening the banner and then changing the consent to allow (this being the 2nd test added)

egekorkan commented 2 months ago

Ah my bad, I ran npx playwright test which ran for other browsers as well. Merging!