-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Issues/update facebook sharing #9867
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
Changes from all commits
1e8623b
620ddb3
6856967
170fe82
5a35eb5
55eb943
e84c448
658f604
4a1de8c
96dc32c
ba54dd7
a79505e
1c59a17
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,15 @@ import WordPressShared | |
| @objc var immutableHandler: ImmuTableViewHandler! | ||
| @objc var delegate: SharingAccountSelectionDelegate? | ||
|
|
||
| fileprivate lazy var noResultsViewController: NoResultsViewController = { | ||
| let controller = NoResultsViewController.controller() | ||
| controller.view.frame = view.frame | ||
| addChildViewController(controller) | ||
| view.addSubview(controller.view) | ||
| controller.didMove(toParentViewController: self) | ||
| return controller | ||
| }() | ||
|
|
||
|
|
||
| // MARK: - Lifecycle Methods | ||
|
|
||
|
|
@@ -72,6 +81,25 @@ import WordPressShared | |
| } | ||
|
|
||
|
|
||
| fileprivate func showNoResultsViewController() { | ||
| let title = NSLocalizedString("No Accounts Found", | ||
| comment: "Title of an error message. There were no third-party service accounts found to setup sharing.") | ||
| let message = NSLocalizedString("Sorry. The social service did not tell us which account could be used for sharing.", | ||
| comment: "An error message shown if a third-party social service does not specify any accounts that an be used with publicize sharing.") | ||
| noResultsViewController.configure(title: title, buttonTitle: nil, subtitle: message, image: nil, accessoryView: nil) | ||
| } | ||
|
|
||
|
|
||
| fileprivate func showFacebookNotice() { | ||
| let message = NSLocalizedString("The Facebook connection could not be made because this account does not have access to any pages. Facebook supports sharing connections to Facebook Pages, but not to Facebook Profiles.", | ||
| comment: "Error message shown to a user who is trying to share to Facebook but does not have any available Facebook Pages.") | ||
|
|
||
| let buttonTitle = NSLocalizedString("Learn more", comment: "A button title.") | ||
| noResultsViewController.configure(title: "", buttonTitle: buttonTitle, subtitle: message, image: nil, accessoryView: nil) | ||
| noResultsViewController.delegate = self | ||
| } | ||
|
|
||
|
|
||
| // MARK: - View Model Wrangling | ||
|
|
||
|
|
||
|
|
@@ -84,6 +112,15 @@ import WordPressShared | |
| var connectedAccounts = [KeyringAccount]() | ||
| var accounts = keyringAccountsFromKeyringConnections(keyringConnections) | ||
|
|
||
| if accounts.count == 0 { | ||
| if publicizeService.serviceID == PublicizeService.facebookServiceID { | ||
| showFacebookNotice() | ||
| } else { | ||
| showNoResultsViewController() | ||
| } | ||
| return ImmuTable(sections: []) | ||
| } | ||
|
|
||
| // Filter out connected accounts into a different Array | ||
| for (idx, acct) in accounts.enumerated() { | ||
| if accountIsConnected(acct) { | ||
|
|
@@ -196,7 +233,11 @@ import WordPressShared | |
|
|
||
| for connection in connections { | ||
| let acct = KeyringAccount(name: connection.externalDisplay, externalID: nil, externalIDForConnection: connection.externalID, keyringConnection: connection) | ||
| accounts.append(acct) | ||
|
|
||
| // Do not include the service if it only supports external users. | ||
| if !publicizeService.externalUsersOnly { | ||
| accounts.append(acct) | ||
| } | ||
|
|
||
| for externalUser in connection.additionalExternalUsers { | ||
| let acct = KeyringAccount(name: externalUser.externalName, externalID: externalUser.externalID, externalIDForConnection: externalUser.externalID, keyringConnection: connection) | ||
|
|
@@ -283,3 +324,13 @@ import WordPressShared | |
| func didDismissSharingAccountViewController(_ controller: SharingAccountViewController) | ||
| func sharingAccountViewController(_ controller: SharingAccountViewController, selectedKeyringConnection keyringConnection: KeyringConnection, externalID: String?) | ||
| } | ||
|
|
||
|
|
||
| extension SharingAccountViewController: NoResultsViewControllerDelegate { | ||
| func actionButtonPressed() { | ||
| if let url = URL(string: "https://en.support.wordpress.com/publicize/#facebook-pages") { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aerych do you think we should show the blog post instead?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should match the behavior in Calypso where the support page is linked from the message shown when attempting to link a Facebook account with no pages. The blog post is linked from the message shown for a pre-existing connection that needs to be disconnected. Unless I somehow got that backwards 😬 |
||
| UIApplication.shared.open(url) | ||
| } | ||
| dismiss(animated: true, completion: nil) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,46 @@ - (void)viewDidLoad | |
|
|
||
| #pragma mark - Instance Methods | ||
|
|
||
| - (void)openFacebookFAQ | ||
| { | ||
| NSURL *url = [NSURL URLWithString:@"https://en.blog.wordpress.com/2018/07/23/sharing-options-from-wordpress-com-to-facebook-are-changing/"]; | ||
| [[UIApplication sharedApplication] openURL:url options:[NSDictionary new] completionHandler:nil]; | ||
| } | ||
|
|
||
| - (NSString *)textForFacebookFooter | ||
| { | ||
| NSString *title = NSLocalizedString(@"As of August 1, 2018, Facebook no longer allows direct sharing of posts to Facebook Profiles. Connections to Facebook Pages remain unchanged.", @"Message shown to users who have an old publicize connection to a facebook profile."); | ||
| return [NSString stringWithFormat:title, self.publicizeConnection.label]; | ||
| } | ||
|
|
||
| - (NSString *)textForBrokenConnectionFooter | ||
| { | ||
| NSString *title = NSLocalizedString(@"There is an issue connecting to %@. Reconnect to continue publicizing.", @"Informs the user about an issue connecting to the third-party sharing service. The `%@` is a placeholder for the service name."); | ||
| return [NSString stringWithFormat:title, self.publicizeConnection.label]; | ||
| } | ||
|
|
||
| - (void)configureReconnectCell: (UITableViewCell *)cell | ||
| { | ||
| cell.textLabel.text = NSLocalizedString(@"Reconnect", @"Verb. Text label. Tapping attempts to reconnect a third-party sharing service to the user's blog."); | ||
| [WPStyleGuide configureTableViewActionCell:cell]; | ||
| cell.textLabel.textAlignment = NSTextAlignmentCenter; | ||
| cell.textLabel.textColor = [WPStyleGuide jazzyOrange]; | ||
| } | ||
|
|
||
| - (void)configureLearnMoreCell: (UITableViewCell *)cell | ||
| { | ||
| cell.textLabel.text = NSLocalizedString(@"Learn More", @"Title of a button. Tapping allows the user to learn more about the specific error."); | ||
| [WPStyleGuide configureTableViewActionCell:cell]; | ||
| cell.textLabel.textAlignment = NSTextAlignmentCenter; | ||
| cell.textLabel.textColor = [WPStyleGuide wordPressBlue]; | ||
| } | ||
|
|
||
| - (void)configureDisconnectCell: (UITableViewCell *)cell | ||
| { | ||
| cell.textLabel.text = NSLocalizedString(@"Disconnect", @"Verb. Text label. Tapping disconnects a third-party sharing service from the user's blog."); | ||
| [WPStyleGuide configureTableViewDestructiveActionCell:cell]; | ||
| } | ||
|
|
||
| - (NSManagedObjectContext *)managedObjectContext | ||
| { | ||
| return self.blog.managedObjectContext; | ||
|
|
@@ -70,7 +110,7 @@ - (NSManagedObjectContext *)managedObjectContext | |
|
|
||
| - (NSInteger)numberOfSectionsInTableView:(UITableView *)tableView | ||
| { | ||
| if ([self.publicizeConnection isBroken]) { | ||
| if ([self.publicizeConnection requiresUserAction]) { | ||
| return 3; | ||
| } | ||
|
|
||
|
|
@@ -96,10 +136,17 @@ - (NSString *)tableView:(UITableView *)tableView titleForFooterInSection:(NSInte | |
| if (section == 0) { | ||
| return NSLocalizedString(@"Allow this connection to be used by all admins and users of your site.", @""); | ||
| } | ||
| if (section == 1 && [self.publicizeConnection isBroken]) { | ||
| NSString *title = NSLocalizedString(@"There is an issue connecting to %@. Reconnect to continue publicizing.", @"Informs the user about an issue connecting to the third-party sharing service. The `%@` is a placeholder for the service name."); | ||
| return [NSString stringWithFormat:title, self.publicizeConnection.label]; | ||
|
|
||
| if (section == 1) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest extracting each logical branch to a separate method. That way |
||
| if ([self.publicizeConnection mustDisconnectFacebook]) { | ||
| return [self textForFacebookFooter]; | ||
| } | ||
|
|
||
| if ([self.publicizeConnection isBroken]) { | ||
| return [self textForBrokenConnectionFooter]; | ||
| } | ||
| } | ||
|
|
||
| return nil; | ||
| } | ||
|
|
||
|
|
@@ -120,14 +167,13 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N | |
| cell = [self switchTableViewCell]; | ||
|
|
||
| } else if (indexPath.section == 1 && [self.publicizeConnection isBroken]) { | ||
| cell.textLabel.text = NSLocalizedString(@"Reconnect", @"Verb. Text label. Tapping attempts to reconnect a third-party sharing service to the user's blog."); | ||
| [WPStyleGuide configureTableViewActionCell:cell]; | ||
| cell.textLabel.textAlignment = NSTextAlignmentCenter; | ||
| cell.textLabel.textColor = [WPStyleGuide jazzyOrange]; | ||
| [self configureReconnectCell:cell]; | ||
|
|
||
| } else if (indexPath.section == 1 && [self.publicizeConnection mustDisconnectFacebook]) { | ||
| [self configureLearnMoreCell:cell]; | ||
|
|
||
| } else { | ||
| cell.textLabel.text = NSLocalizedString(@"Disconnect", @"Verb. Text label. Tapping disconnects a third-party sharing service from the user's blog."); | ||
| [WPStyleGuide configureTableViewDestructiveActionCell:cell]; | ||
| [self configureDisconnectCell:cell]; | ||
| } | ||
|
|
||
| return cell; | ||
|
|
@@ -139,7 +185,8 @@ - (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath | |
|
|
||
| if (indexPath.section == 1 && [self.publicizeConnection isBroken]) { | ||
| [self reconnectPublicizeConnection]; | ||
|
|
||
| } else if (indexPath.section == 1 && [self.publicizeConnection mustDisconnectFacebook]) { | ||
| [self openFacebookFAQ]; | ||
| } else { | ||
| [self promptToConfirmDisconnect]; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a completely personal preference, but I tend to declare strings as constants (or as static cases in an enumeration), so that, at the call site, like here, it would read similar to this:
I have found that makes the code slightly easier to parse for me. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a fan of that when a string is reused. When used only once I like declaring a string in the context of its usage as a way of having as much context as possible in one place.