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

Full screen comments #10140

Merged
merged 10 commits into from Jul 11, 2019

Conversation

@planarvoid
Copy link
Contributor

commented Jul 3, 2019

Fixes #9942

This PR adds a functionality to expand a comment field to a full-screen view. This is useful when there are long comments that were previously limited by the 4 lines of input height in the Comments detail screen.

This PR is mostly based on your changes @theck13. I've added a section start and section end params to the comment dialog fragment (I think it makes sense to keep the cursor position when we expand the comment) and I've fixed the submit button when sending a comment from the reader. It's now correctly enabled when you type in a text and disabled when you remove the text.

To test:

  • Go to Reader/Post
  • Click on the Comments on a Post
  • The Reader comment screen opens
    • Notice that there is an expand button on the left side of the input field
    • Notice that the submit button is disabled
  • Write a text in the input field
    • Notice that the submit button is now enabled
  • Click on the "Expand" button
    • The comment expands to a full-screen view
    • Notice that the cursor position is kept
  • Click on the "Collapse" button
    • The comment collapses back to the inline view
    • Notice that the cursor position is kept
  • Click on the "Submit" button
  • Comment is posted
    • Notice that the submit button is disabled again

To test 2:

  • Go to My Site/Comments
  • Click on a Comment
  • The Comment Detail screen opens
    • Notice that there is an expand button on the left side of the input field
    • Notice that the submit button is disabled
  • Write a text in the input field
    • Notice that the submit button is now enabled
  • Click on the "Expand" button
    • The comment expands to a full-screen view
    • Notice that the cursor position is kept
  • Click on the "Collapse" button
    • The comment collapses back to the inline view
    • Notice that the cursor position is kept
  • Click on the "Submit" button
  • Comment is posted
    • Notice that the submit button is disabled again

untitled

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@planarvoid planarvoid changed the title Feature/full screen comments Full screen comments Jul 3, 2019

@planarvoid planarvoid requested a review from theck13 Jul 3, 2019

@planarvoid planarvoid self-assigned this Jul 3, 2019

@planarvoid planarvoid added the Comments label Jul 3, 2019

@planarvoid planarvoid added this to the 12.9 milestone Jul 3, 2019

@peril-wordpress-mobile

This comment has been minimized.

Copy link

commented Jul 3, 2019

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@wordpress-mobile wordpress-mobile deleted a comment from wpmobilebot Jul 3, 2019

planarvoid added some commits Jul 3, 2019

@theck13
Copy link
Contributor

left a comment

Thanks for taking care of this, @planarvoid! The code and updates look good to me. Good idea on the selection improvements! I have only one comment and one enhancement.

When collapsing the full-screen view on the Sites > Comments screen, the selection isn't retained. The selection is retained when doing the same thing on the Reader screen though. See the animation below for illustration.

We could add a hint message and haptic feedback when long-pressing the expand and send buttons in the collapsed view to the CommentDetailFragment and ReaderCommentListActivity classes. That would mimic what happens when you long-press an action in the top app bar and may help if users don't know what those icons mean. That could would be something like this.

buttonExpand.setOnLongClickListener(new OnLongClickListener() {
    @Override public boolean onLongClick(View view) {
        if (view.isHapticFeedbackEnabled()) {
            view.performHapticFeedback(HapticFeedbackConstants.LONG_PRESS);
        }

        Toast.makeText(view.getContext(), R.string.description_expand, Toast.LENGTH_SHORT).show();
        return true;
    }
});
mSubmitReplyBtn.setOnLongClickListener(new OnLongClickListener() {
    @Override public boolean onLongClick(View view) {
        if (view.isHapticFeedbackEnabled()) {
            view.performHapticFeedback(HapticFeedbackConstants.LONG_PRESS);
        }

        Toast.makeText(view.getContext(), R.string.send, Toast.LENGTH_SHORT).show();
        return true;
    }
});

Neither of those are required to merge these changes though. We can create new issues for them if you think that's better.

@peril-wordpress-mobile

This comment has been minimized.

Copy link

commented Jul 10, 2019

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@planarvoid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

@theck13 thanks for the review and the good tips! I've fixed the selection issue and added the toast feedback to both expand and send actions as you mentioned. It should be ready for another check.

@theck13 theck13 merged commit 7eb6113 into develop Jul 11, 2019

4 checks passed

Peril Found some issues. Don't worry, everything is fixable.
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: strings-check Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details

@theck13 theck13 deleted the feature/full_screen_comments branch Jul 11, 2019

@planarvoid planarvoid referenced this pull request Jul 11, 2019
1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.