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

UHF-5047 map visibility cookie consent #540

Merged
merged 5 commits into from
Feb 23, 2023

Conversation

dire
Copy link
Contributor

@dire dire commented Feb 7, 2023

UHF-5047

Maps were shown even though cookie consent was not given.

What was done

  • Display a similar cookie consent screen for maps than what videos have before the cookies are accepted.
  • To reduce duplicate code, a common template was created to show cookie blocker in embedded medias
  • CSS/template changes had to be done after adding the cookie blocker.

How to install

  • Make sure any instance is up and running on latest dev branch.
    • git pull origin dev
  • Update the HDBT theme
    • composer require drupal/hdbt:dev-UHF-5047_Map-visibility-cookie-consent
  • Run make drush-cr

How to test

  • Add a map component to any page.
  • See that the map is not shown unless cookies are accepted.
  • Check that the map host is shown properly in the text on the blocker screen.
  • Check that the styling is still same for the map component.
  • Since the templates for the remote videos are affected, check that the video component works still fine.

Designers review

  • This PR does not need designers review
  • This PR has been visually reviewed by a designer (Name of the designer)

@Arkkimaagi Arkkimaagi self-requested a review February 23, 2023 06:32
Copy link
Contributor

@Arkkimaagi Arkkimaagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost. :)

Please see my comments below.

This branch is missing the changed dist folder files. Please create them before merging.

src/js/embedded-content-cookie-compliance.js Outdated Show resolved Hide resolved
src/js/embedded-content-cookie-compliance.js Outdated Show resolved Hide resolved
src/js/embedded-content-cookie-compliance.js Show resolved Hide resolved
templates/misc/embedded-content-cookie-compliance.twig Outdated Show resolved Hide resolved
@dire dire force-pushed the UHF-5047_Map-visibility-cookie-consent branch from 34b268e to e1c5c04 Compare February 23, 2023 07:41
@dire
Copy link
Contributor Author

dire commented Feb 23, 2023

Good catches! All should be fixed now.

Copy link
Contributor

@Arkkimaagi Arkkimaagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works now, good job!

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.

2 participants