Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Fix #7882: Enable cookie consent notice blocking by default #7883

Merged
merged 2 commits into from
Aug 16, 2023
Merged

Fix #7882: Enable cookie consent notice blocking by default #7883

merged 2 commits into from
Aug 16, 2023

Conversation

cuba
Copy link
Contributor

@cuba cuba commented Aug 16, 2023 •
edited by soner-yuksel
Loading

Summary of Changes

This pull request fixes #7882

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()
  • New or updated UI has been tested across:
    • Light & dark mode
    • Different size classes (iPhone, landscape, iPad)
    • Different dynamic type sizes

Test Plan:

See issue for STR

Also note for QA : Please check Cookie Consent Popover is not presented

Reviewer Checklist:

  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate unit test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable).
  • Issue and pull request is assigned to a milestone (should happen at merge time).

@cuba cuba requested a review from a team as a code owner August 16, 2023 04:44
@cuba cuba self-assigned this Aug 16, 2023
@cuba cuba force-pushed the js/7882-enable-cookie-consent-blocking-by-default branch from 547d19f to 507c68e Compare August 16, 2023 04:50
@soner-yuksel
Copy link
Contributor

soner-yuksel commented Aug 16, 2023 •
edited
Loading

Please add the issue to description under This pull request fixes #7882
Fix the title of the PR by including Fix #7882: Enable cookie consent notice blocking by default

And Please FIX the tests, CI is giving error and saying cookieconsent doesnt exist anymore in Tests!

soner-yuksel
soner-yuksel suggested changes Aug 16, 2023
View reviewed changes
Copy link
Contributor

@soner-yuksel soner-yuksel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please remove the GIFImage, it is not used anymore and we should not use in future. (Don't accept GIF from designer to be displayed somewhere in the app)

@cuba
Copy link
Contributor Author

cuba commented Aug 16, 2023

Please remove the GIFImage, it is not used anymore and we should not use in future. (Don't accept GIF from designer to be displayed somewhere in the app)

Right forgot about this file. Thanks for the reminder

@cuba cuba changed the title Enable cookie consent notice blocking by default Fix #7882: Enable cookie consent notice blocking by default Aug 16, 2023
@cuba
Copy link
Contributor Author

cuba commented Aug 16, 2023

Please add the issue to description under This pull request fixes #7882 Fix the title of the PR by including Fix #7882: Enable cookie consent notice blocking by default

Done

And Please FIX the tests, CI is giving error and saying cookieconsent doesnt exist anymore in Tests!

Ahh sorry did this late last night and didn't want to scrap my brave-core changes so I did it in the dark.

@cuba cuba requested a review from soner-yuksel August 16, 2023 17:42
@cuba cuba added the QA/Yes label Aug 16, 2023
@soner-yuksel soner-yuksel added this to the 1.57 milestone Aug 16, 2023
@cuba cuba merged commit 6c17b53 into development Aug 16, 2023
@cuba cuba deleted the js/7882-enable-cookie-consent-blocking-by-default branch August 16, 2023 21:03
arthuredelstein pushed a commit to brave/brave-core that referenced this pull request Feb 13, 2024
@cuba
Fix brave/brave-ios#7882: Enable cookie consent notice blocking by de…
6b9200e 
…fault (brave/brave-ios#7883)

* Enable cookie consent notice blocking by default

* Fix tests
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@soner-yuksel soner-yuksel soner-yuksel approved these changes

Assignees

@cuba cuba

Labels
QA/Yes
Projects
None yet
Milestone
1.57
Development

Successfully merging this pull request may close these issues.

Enable cookie consent notice blocking by default
2 participants
@cuba @soner-yuksel