Issue 3165 - The configureable whitelist #11
Conversation
Merge with Eyeo upstream
Sync with Eyeo
Sync with Eyeo
Sync with Eyeo
Sync with Eyeo
Sync with Eyeo
Change-Id: ee5de24cf5 Story-Id: 1431320/stories/105213900
| @@ -0,0 +1,18 @@ | |||
| # Uncomment this line to define a global platform for your project | |||
There was a problem hiding this comment.
Project is using cocoapods now. Cocoapods is managing external libraries, which does not have to populate repository.
pod install needs to be called before project is build.
|
@fhd: This PR is ready for review. It includes new whitelist functionality without sharing extension. |
| cell = [tableView dequeueReusableCellWithIdentifier:@"WebsiteCell" forIndexPath:indexPath]; | ||
| cell.textLabel.text = self.adblockPlus.whitelistedWebsites[indexPath.row]; | ||
|
|
||
| UIButton *buttom = [UIButton buttonWithType:UIButtonTypeCustom]; |
There was a problem hiding this comment.
"buttom" looks like a typo (also in "onTrashButtomTouched").
|
@dedecej Sorry for the delay! Had a look now (except for the parsing/merging code, saving that for later). Looks good all in all, left a few comments/questions. |
|
@dedecej Added one more relevant comment, plus, I think you might have missed this one: #11 (comment) As I said, I haven't reviewed the merging code yet, we should first finish discussing how that should work. |
| @@ -0,0 +1,21 @@ | |||
| { | |||
There was a problem hiding this comment.
Why is there Image.imageset in addition to trash.imageset? They seem to be about the same image.
|
@dedecej Reviewed everything now. Looks good by and large, mostly style stuff. |
|
@fhd: We are good to go. I addressed all of your issues. |
| @@ -33,7 +33,7 @@ - (void)viewDidLoad | |||
| { | |||
| [super viewDidLoad]; | |||
|
|
|||
| // Test if adblock browser is installed | |||
| // Test if adblock browser is installed | |||
There was a problem hiding this comment.
Unrelated, but to speed things up I'll leave it out when merging this and apply it as a separate Noissue.
|
@dedecej Yeah, looks pretty good now, just some really small stuff remaining, and I'll merge it. |
|
@fhd: Those changes were small, PR is ready. |
|
LGTM! Merging it. |
|
Merged: 5d48434, thanks again! |
Related to issue 3165.