[Remoting Android] Separate help and feedback in navigation drawer

[Remoting Android] Separate help and feedback in navigation drawer

This CL separates the help and feedback items in the navigation drawer
according to the UX review feedback.

BUG=650813
Committed: https://crrev.com/8ec3ca27163b6e932a03a304e0a7aad7d6a884e4
Cr-Commit-Position: refs/heads/master@{#421716}

[Yuwei, 2016-09-29 00:05:00]

yuweih@chromium.org changed reviewers:
+ joedow@chromium.org, lambroslambrou@chromium.org

[Yuwei, 2016-09-29 00:05:01]

PTAL

https://codereview.chromium.org/2381543003/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/help/FeedbackSender.java
(right):

https://codereview.chromium.org/2381543003/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/help/FeedbackSender.java:25:
public class FeedbackSender {
We may want to have an internal feedback implementation according to
go/feedback-gcore

https://codereview.chromium.org/2381543003/diff/1/remoting/resources/remoting...
File remoting/resources/remoting_strings.grd (right):

https://codereview.chromium.org/2381543003/diff/1/remoting/resources/remoting...
remoting/resources/remoting_strings.grd:595: <message desc="Android action-bar
menu item for showing the Help screen" name="IDS_ACTIONBAR_HELP"
formatter_data="android_java">
Now IDS_ACTIONBAR_HELP and IDS_ACTIONBAR_HELP_TITLE are the same thing. Not sure
whether we want to remove IDS_ACTIONBAR_HELP_TITLE?

[Lambros, 2016-09-29 00:18:39]

There's no real need to implement the public side of sending feedback, if you
update the internal implementation (which should be trivial).
Since you've done the work anyway, I guess there's no harm in keeping it?
I notice that we pass in a HelpContext, which is just a string. This simplifies
the process - we could use the special string "feedback" to mean "just launch
the feedback directly". Then we don't have to do the full 3-step dance that I
described in the bug-report.

[Lambros, 2016-09-29 00:29:24]

lgtm

https://codereview.chromium.org/2381543003/diff/1/remoting/android/java/res/m...
File remoting/android/java/res/menu/feedback_list_item.xml (right):

https://codereview.chromium.org/2381543003/diff/1/remoting/android/java/res/m...
remoting/android/java/res/menu/feedback_list_item.xml:1: <?xml version="1.0"
encoding="utf-8"?>
Do we need a separate XML file for each menu-item?
Not for this CL, but we should consider refactoring this.

https://codereview.chromium.org/2381543003/diff/1/remoting/resources/remoting...
File remoting/resources/remoting_strings.grd (right):

https://codereview.chromium.org/2381543003/diff/1/remoting/resources/remoting...
remoting/resources/remoting_strings.grd:589: <message desc="Android action-bar
menu item for sending feedback" name="IDS_ACTIONBAR_SEND_FEEDBACK"
formatter_data="android_java">
Remove 'action-bar', maybe replace with 'application'.
Maybe expand:
.. for sending product feedback to the developers.

https://codereview.chromium.org/2381543003/diff/1/remoting/resources/remoting...
remoting/resources/remoting_strings.grd:595: <message desc="Android action-bar
menu item for showing the Help screen" name="IDS_ACTIONBAR_HELP"
formatter_data="android_java">
Remove 'action-bar', maybe replace with 'application'.

[Yuwei, 2016-09-29 01:04:47]

Thanks!

Next step I'll figure out the best way to change the HelpAndFeedback interface.

https://codereview.chromium.org/2381543003/diff/1/remoting/android/java/res/m...
File remoting/android/java/res/menu/feedback_list_item.xml (right):

https://codereview.chromium.org/2381543003/diff/1/remoting/android/java/res/m...
remoting/android/java/res/menu/feedback_list_item.xml:1: <?xml version="1.0"
encoding="utf-8"?>
On 2016/09/29 00:29:24, Lambros wrote:
> Do we need a separate XML file for each menu-item?
> Not for this CL, but we should consider refactoring this.

Acknowledged. Added a TODO

https://codereview.chromium.org/2381543003/diff/1/remoting/resources/remoting...
File remoting/resources/remoting_strings.grd (right):

https://codereview.chromium.org/2381543003/diff/1/remoting/resources/remoting...
remoting/resources/remoting_strings.grd:589: <message desc="Android action-bar
menu item for sending feedback" name="IDS_ACTIONBAR_SEND_FEEDBACK"
formatter_data="android_java">
On 2016/09/29 00:29:24, Lambros wrote:
> Remove 'action-bar', maybe replace with 'application'.
> Maybe expand:
> .. for sending product feedback to the developers.

Done.

https://codereview.chromium.org/2381543003/diff/1/remoting/resources/remoting...
remoting/resources/remoting_strings.grd:595: <message desc="Android action-bar
menu item for showing the Help screen" name="IDS_ACTIONBAR_HELP"
formatter_data="android_java">
On 2016/09/29 00:29:24, Lambros wrote:
> Remove 'action-bar', maybe replace with 'application'.

Done.

[Yuwei, 2016-09-29 01:04:57]

The CQ bit was checked by yuweih@chromium.org

[Yuwei, 2016-09-29 01:04:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
lambroslambrou@chromium.org
Link to the patchset: https://codereview.chromium.org/2381543003/#ps40001
(title: "Reviewer's Feedback")

[commit-bot: I haz the power, 2016-09-29 01:05:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 02:01:04]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-29 02:03:32]

Description was changed from

==========
[Remoting Android] Separate help and feedback in navigation drawer

This CL separates the help and feedback items in the navigation drawer
according to the UX review feedback.

BUG=650813
==========

to

==========
[Remoting Android] Separate help and feedback in navigation drawer

This CL separates the help and feedback items in the navigation drawer
according to the UX review feedback.

BUG=650813

Committed: https://crrev.com/8ec3ca27163b6e932a03a304e0a7aad7d6a884e4
Cr-Commit-Position: refs/heads/master@{#421716}
==========

[commit-bot: I haz the power, 2016-09-29 02:03:33]

Patchset 3 (id:??) landed as
https://crrev.com/8ec3ca27163b6e932a03a304e0a7aad7d6a884e4
Cr-Commit-Position: refs/heads/master@{#421716}