This is looking good!
The one thing that stood out once I went through this is that this is really three changes in one:

General support for tab-based shortcuts
The shortcut dialog
The tab support for review request fields

It'd really help to split them up so it's easier to test and land parts. Especially since I think the tab-based shortcuts are pretty close to ready, the dialog is close but need some CSS Component work, and there's still going to be a lot of work needed for the review request fields to tie them into the extensible field support we have.

We're trying to get rid of most of common.less and move to the CSS Component structure. This would be best in a rb/css/ui/shortcuts-dialog.less, as a .rb-c-shortcuts-dialog component, using effective style names.
I know we're at the end of the term, but if you're interested in working on this after the semester ends, this would be a great task for that time! :) Otherwise consider it a reminder for us before it lands.

Can you wrap this in gettext() so the string can be localized?

Looks like this would conflict with the other review request, right? That uses "docs," this uses "description."

You can use $.ui.keyCode.TAB and 'H'.charCodeAt(0), respectively.
It'd also be nice to be consistent with "KeyCode", rather than "Keycode".

You can make use of chaining here:

pageView.$el
    .trigger(....)
    .trigger(....)
    ....;

It'd be nice to make these private methods (prefix with a _).

This should use _.isEmpty().

These must be localized. You'll want to pass in the localized strings as parameters to the template.

Right now, we should be using _.each() instead of Object.entries().forEach(). It has wider browser compatibility.
We also cannot use fat arrow functions in templates. They'll be preserved and sent to the browser, and will break browsers without the support (not uncommon in enterprises).
Basically, anything in a template skips Babel transpilation, so we can't use unsafe stuff.

You should use if (entry[1].tabBaseCallback). We want to test if it's present and has a value, and this is the safest, shortest way. (It will be undefined if missing.)

Same here.

We have to use var here, for browser compatibility.

Missing a semicolon.

This also needs to be localized.

This should be const (and appear as the first variable -- we do defined variables and consts before undefined and lets), since it won't be redefined.

I don't think we need to call .render() anymore. .show() does this for us, iirc.

We don't re-define this, so we should use const.

Comments should go on their own line and be proper sentences (capitalization, punctuation, "the", etc.).

Let's use _.size() here. It will guarantee the right thing for the browser it's running on.

Two things that can improve this:

Let's pull out innerText into a variable, so we can check it without re-fetching it each time.
Let's use expect(...).toContain('test1'), so we can get better output if the expectation fails.

Mind adding a comment about how this state is managed?

We should just reference $.ui.keyCode.TAB.

Instead of _.bind(), we should use =>.

You can chain these:

this.$el
    .on(...)
    .on(...)
    ...;


The binding below can also be included in the chain.

This is missing an "Args" section, documenting evt and isTabBasedShortcut.

Docstring summaries must be a single line.
How about:

/**
 * Call a shortcut handler when a key or combination is pressed.

This can be:

const keyBinding = this.keyBindings[keys];

let func = (isTabBasedShortcut
            ? keyBinding.tabBasedShortcut
            : keyBinding.callback);

Typo: "field"

Can you place this alphabetically in the list of test files?