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

Fix #4009: Disable History sync when user switches to PB only #4013

Merged
merged 3 commits into from
Aug 9, 2021

Conversation

soner-yuksel
Copy link
Contributor

@soner-yuksel soner-yuksel commented Aug 6, 2021

History Private Browsing Only overlay is added in case PB Only Mode is enabled.
And hiding the "Delete All" button.

Summary of Changes

This pull request fixes #4009

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()

Test Plan:

Check History When B Only mode is enabled and sync items are being seen or an overlay is hown

Screenshots:

16

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).

@soner-yuksel soner-yuksel added this to the 1.30 milestone Aug 6, 2021
@soner-yuksel soner-yuksel requested a review from a team August 6, 2021 16:10
@soner-yuksel soner-yuksel self-assigned this Aug 6, 2021
Copy link
Contributor

@iccub iccub left a comment

Choose a reason for hiding this comment

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

Maybe hide the delete-all button too?

@soner-yuksel
Copy link
Contributor Author

Maybe hide the delete-all button too?

You are totally right, we should hide the "Delete".

Change is at b944cbe

@soner-yuksel soner-yuksel requested a review from iccub August 6, 2021 18:06
}

override func viewWillAppear(_ animated: Bool) {
super.viewWillAppear(animated)

guard !Preferences.Privacy.privateBrowsingOnly.value else {
Copy link
Contributor

Choose a reason for hiding this comment

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

convert to if this is too hard to read imo, two negations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I always try to guard the initial conditions instead of showing them inside If clause but this particular conditions, you are right it is very hard to read.

Change at 1125af9

@soner-yuksel soner-yuksel requested a review from iccub August 9, 2021 14:43
@soner-yuksel soner-yuksel merged commit e1d902f into development Aug 9, 2021
@soner-yuksel soner-yuksel deleted the fix/pb-history-show branch August 9, 2021 15:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable History sync when user switches to PB only
2 participants