Skip to content
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

Comment Detail: Create the initial view #17088

Merged
merged 6 commits into from
Aug 30, 2021

Conversation

dvdchr
Copy link
Contributor

@dvdchr dvdchr commented Aug 26, 2021

Refs #17087

Summary

This adds the initial view for comment details, along with the feature flag. Some notes for review:

  • The feature flag is wired in My Site > Comments, so you don't have to manually change the code.
  • The new view is still empty, since more PRs are coming to add on this one. But I've wired in the "Edit" button to connect to the new EditCommentTableViewController component developed in Comments: add new Edit view #17000

To test

Scenario 1: Ensure flow correctness with flag disabled

  • Go to My Site > Comments, and tap one of the comments.
  • Ensure that the previous comment details screen is shown and works as expected.

Scenario 2: The new detail screen should be reachable with flag enabled.

  • Enable the New Comment Details feature flag.
  • Go to My Site > Comments. Tap one of the comments.
  • Verify that the (empty) new view is shown 😅
  • Verify that the Edit button correctly shows the new Edit Comment screen (although it's still incomplete, progress for this is tracked in Comments: add new Edit view #17000).

Regression Notes

  1. Potential unintended areas of impact
    Current comment details is not shown correctly when tapped from My Site > Comments.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manually tested that the implementation is correct.

  3. What automated tests I added (or what prevented me from doing so)
    n/a, the feature is behind a disabled flag.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@dvdchr dvdchr added this to the 18.2 milestone Aug 26, 2021
@dvdchr dvdchr requested review from aerych and ScoutHarris August 26, 2021 13:01
@dvdchr dvdchr self-assigned this Aug 26, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Aug 26, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Aug 26, 2021

You can trigger an installable build for these changes by visiting CircleCI here.

Copy link
Contributor

@ScoutHarris ScoutHarris left a comment

Choose a reason for hiding this comment

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

Hey @dvdchr . Just a question about a bit of code that looks unused. Otherwise 👍 !

:shipit:

// MARK: - Localization

private extension String {
static let editBarButtonTitle = NSLocalizedString("Edit", comment: "Edits a Comment")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. I forgot to clean it up. Thanks for noticing this! 😄 . Removed in ff524a7.

CommentViewController *vc = [CommentViewController new];
vc.comment = comment;
Comment *comment = [self.tableViewHandler.resultsController objectAtIndexPath:indexPath];
UIViewController *detailViewController;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@dvdchr dvdchr enabled auto-merge August 30, 2021 09:57
@dvdchr dvdchr merged commit 3a84457 into develop Aug 30, 2021
@dvdchr dvdchr deleted the issue/17087-new-comment-details-view branch August 30, 2021 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants