1231954 - Add header link for keyboard shortcut reference

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

People want to use MozReview with keyboard shortcuts, but finding a reference to those shortcuts is not simple. We should add a header link to take people to a keyboard shortcut reference (or to pop up an in-content help overlay with keyboard shortcut information without taking people off the current page).

Comment 1

[David Walsh :davidwalsh]

What's the URL for the shortcuts detail page?

Comment 2

[Steven MacLeod [:smacleod]]

(In reply to David Walsh :davidwalsh from comment #1)
> What's the URL for the shortcuts detail page?

https://www.reviewboard.org/docs/manual/2.5/users/reviews/reviewing-diffs/#keyboard-shortcuts

Comment 3

[Salvador de la Puente González [:salva]]

Hey, David, are you looking at this right now? I think I could solve it if you're not working on it (if so, please assign to you).

Comment 4

[David Walsh :davidwalsh]

All yours! :)

Comment 5

[David Walsh :davidwalsh]

You still on this Salva?

Comment 6

[Salvador de la Puente González [:salva]]

Actually, I did not start as I'm with bug 1255876 yet. Feel free to take it if you want.

Comment 7

[David Walsh :davidwalsh]

Attachment: MozReview: Add link for keyboard shortcuts (Bug 1231954).

Review commit: https://reviewboard.mozilla.org/r/61070/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61070/

Comment 8

[:glob ✱]

Comment on attachment 8765997 [details]
MozReview: Add link for keyboard shortcuts (Bug 1231954).

https://reviewboard.mozilla.org/r/61070/#review58050

::: reviewboard/reviewboard/templates/base/_nav_support_menu.html:7
(Diff revision 1)
>  
>  <li>
>   <a href="#">{% trans "Support" %} &#9662;</a>
>    <ul>
> -   <li><a href="{{RB_MANUAL_URL}}">{% trans "Documentation" %}</a></li>
> +   <li><a href="{{RB_MANUAL_URL}}" target="_blank">{% trans "Documentation" %}</a></li>
> +   <li><a href="https://www.reviewboard.org/docs/manual/2.5/users/reviews/reviewing-diffs/#keyboard-shortcuts" target="_blank">{% trans "Keyboard Shortcuts" %}</a></li>

please use "{{RB_MANUAL_URL}}users/reviews/reviewing-diffs/#keyboard-shortcuts" instead of hard coding the full url

::: reviewboard/reviewboard/templates/base/_nav_support_menu.html:8
(Diff revision 1)
>  <li>
>   <a href="#">{% trans "Support" %} &#9662;</a>
>    <ul>
> -   <li><a href="{{RB_MANUAL_URL}}">{% trans "Documentation" %}</a></li>
> +   <li><a href="{{RB_MANUAL_URL}}" target="_blank">{% trans "Documentation" %}</a></li>
> +   <li><a href="https://www.reviewboard.org/docs/manual/2.5/users/reviews/reviewing-diffs/#keyboard-shortcuts" target="_blank">{% trans "Keyboard Shortcuts" %}</a></li>
>     <li><a href="{% url 'support' %}">{% trans "Get Support" %}</a></li>

since the other two items in this menu now open in a new tab, 'get support' should do the same.

Comment 9

[David Walsh :davidwalsh]

Comment on attachment 8765997 [details]
MozReview: Add link for keyboard shortcuts (Bug 1231954).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61070/diff/1-2/

Comment 10

[David Walsh :davidwalsh]

https://reviewboard.mozilla.org/r/61070/#review58158

Issues fixed!  THank you!

Comment 11

[:glob ✱]

Comment on attachment 8765997 [details]
MozReview: Add link for keyboard shortcuts (Bug 1231954).

https://reviewboard.mozilla.org/r/61070/#review58170

lgtm

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/webtools/reviewboard/rev/d1a3a686b1e0
MozReview: Add link for keyboard shortcuts .  r=glob

Comment 13

[Steven MacLeod [:smacleod]]

https://reviewboard.mozilla.org/r/61070/#review58932

::: reviewboard/reviewboard/templates/base/_nav_support_menu.html:8
(Diff revision 2)
>  <li>
>   <a href="#">{% trans "Support" %} &#9662;</a>
>    <ul>
> -   <li><a href="{{RB_MANUAL_URL}}">{% trans "Documentation" %}</a></li>
> -   <li><a href="{% url 'support' %}">{% trans "Get Support" %}</a></li>
> +   <li><a href="{{RB_MANUAL_URL}}" target="_blank">{% trans "Documentation" %}</a></li>
> +   <li><a href="{{RB_MANUAL_URL}}users/reviews/reviewing-diffs/#keyboard-shortcuts" target="_blank">{% trans "Keyboard Shortcuts" %}</a></li>
> +   <li><a href="{% url 'support' %}" target="_blank">{% trans "Get Support" %}</a></li>

"When making changes to a built-in review board template, forking the template file and changing the view to reference your new modified file is preferred - this will lessen merge conflicts when rebasing all of our changes on an updated RB (and allow us to integrate new upstream changes to the template in one go)" - https://public.etherpad-mozilla.org/p/glob-1264203

Comment 14

[:glob ✱]

https://reviewboard.mozilla.org/r/61070/#review58932

> "When making changes to a built-in review board template, forking the template file and changing the view to reference your new modified file is preferred - this will lessen merge conflicts when rebasing all of our changes on an updated RB (and allow us to integrate new upstream changes to the template in one go)" - https://public.etherpad-mozilla.org/p/glob-1264203

filed bug 1284494 to track this.