Skip to content

[Stats Refresh] Post Stats: show web view when post title tapped - #11499

Merged
ScoutHarris merged 5 commits into
developfrom
feature/11344-post_stats_title_webview
Apr 18, 2019
Merged

[Stats Refresh] Post Stats: show web view when post title tapped#11499
ScoutHarris merged 5 commits into
developfrom
feature/11344-post_stats_title_webview

Conversation

@ScoutHarris

Copy link
Copy Markdown
Contributor

Fixes #11344

When on Post Stats, tapping the post title will now open a web view for that post.

To test:

  • Access Post Stats via:
    • Insights > Latest Post Summary > View more
    • Period > Posts and Pages > row selection
    • Period > Posts and Pages > View more > row selection
  • Verify tapping in the post title area opens a web view for that post.

post_title

@ScoutHarris ScoutHarris added this to the 12.3 milestone Apr 18, 2019
@ScoutHarris
ScoutHarris requested a review from a user April 18, 2019 18:24
@ScoutHarris ScoutHarris self-assigned this Apr 18, 2019
@ScoutHarris ScoutHarris mentioned this pull request Apr 18, 2019
45 tasks
let changeDispatcher = Dispatcher<Void>()
private var postTitle: String?
private var postURL: URL?
private weak var postStatsDelegate: PostStatsDelegate?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

weak var : 👍

extension PostStatsTableViewController: PostStatsDelegate {

func displayWebViewWithURL(_ url: URL) {
let webViewController = WebViewControllerFactory.controllerAuthenticatedWithDefaultAccount(url: url)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TIL about WebViewControllerFactory 😄

func displayWebViewWithURL(_ url: URL) {
let webViewController = WebViewControllerFactory.controllerAuthenticatedWithDefaultAccount(url: url)
let navController = UINavigationController.init(rootViewController: webViewController)
present(navController, animated: true, completion: nil)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fun fact: the completion handler of present(_:animated:completion:) has a default parameter value of nil. So, you could delete that if you were so inclined.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, cool. Thanks! I took the liberty of removing that param from all the Stats displayWebViewWithURL methods.

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey @ScoutHarris - I left one nitpicky suggestion regarding a default method parameter, but I can confirm the following:

✅ Branch builds & tests pass
✅ I am able to manually step through the outlined sequences and see the web view controller in each of them.

Nice work! :shipit:

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.

Stats: Request to link directly to posts/pages from stats

1 participant