Migrate to managed cookie scripts - #13874
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the custom CookieNotice component with a managed cookie notification bar across the sites, updating the layout files, styles, and client/server options accordingly. The feedback highlights two main issues: first, several new CSS variables used in the shared _cookie-notice.scss are only defined in the www site, which will lead to broken styles on the docs site; second, there is an inconsistency in the cookie consent category configuration (2B vs 2A) between DashLayout and DefaultLayout that should be aligned.
|
Staged preview of the updated docs.flutter.dev site (updated for commit f6a65aa): https://flutter-docs-prod--docs-pr13874-feat-new-cookie-banner-tel7uk1f.web.app |
|
Staged preview of the updated flutter.dev site (updated for commit f6a65aa): https://flutter-dev-230821--www-pr13874-feat-new-cookie-banner-wrub9zxr.web.app |
There was a problem hiding this comment.
Code Review
This pull request replaces the custom, self-hosted cookie notice component with a managed Google cookie notification bar. It updates the layouts (DashLayout and DefaultLayout) to load the managed script and styles, and configures Google Tag Manager (GTM) to initialize only after the cookie banner indicates consent via the glueCookieNotificationBarLoaded callback. Old Google Analytics tracking code and custom cookie notice components have been removed, and theme variables have been updated to style the new managed banner. I have no feedback to provide as there are no review comments.
sfshaza2
left a comment
There was a problem hiding this comment.
I'm going to approve, but won't land until there's consensus, or at least until Parker is ok with it. :D
|
@jesskuras These changes adjust analytics initialization so let me know if you see any changes or regressions once this lands. |
Similar to the dart.dev site in dart-lang/site-www#7500.
The flutter.dev site already used it but didn't have the adapted styles, so it was configured to use fonts we didn't have set up.