Has this been manually tested with other browsers? (i.e. Safari, Firefox, Edge, etc.)

class names are usually kebab case and i think don't typically include view in them. also did you mean collection here and not selection? i might be misunderstanding that part but basically i think the class name should be branches-collection

i was thinking about doing this for my work but i didn't get around to looking up how yet - glad i spotted it here!

iirc there's a constant defined for the enter key code already - i think $.ui.keyCode.ENTER ? also i believe which is preferred over keyCode in jQuery - https://stackoverflow.com/questions/4471582/keycode-vs-which

not sure which key this corresponds to, but same comment above about using an already defined constant and possibly favoring which instead of keyCode

can you do this.views[this._currentElement].focus(); ? same thing as below

since this is the class name of the view, i think $('.branchesSelectionView') could be replaced with this.$el

same issue about which and using an already defined constant here. also, if this supports the tab key (keycode 9) i think this should also support shift+tab to go in the opposite direction (same behaviour as up arrow key)

i think this._currentElement could be better named as this._activeCommitIndex or something similar; the name is kinda confusing. when i saw the constant defined at the top of the file, i thought it was referring to a DOM/jQuery node and not just the index of one

same comment about keyCode and using a constant for the numeric value

does this fn need a check to make sure .rb-c-search-field__input is actually the active element before moving focus to the rbtools tip?

i think Handling should be lowercase to be consistent with the style in other tests

i think CommitsView should be camel cased?

you could put this into a beforeEach

oh yeah these were the ui key code constants i was referring to above! :D

could you use toHaveBeenCalledWith(...) here to check the arguments?

toHaveBeenCalledWith(...) ?

toHaveBeenCalledWith(...) could be used here too i think

toHaveBeenCalledWith(...) ?

i think Handling should be lowercase

could view.render(); and view.delegateEvents(); be put into a beforeEach?

toHaveBeenCalledWith(...) ?

toHaveBeenCalledWith(...) ?

toHaveBeenCalledWith(...) ?

maybe toHaveBeenCalledWith(...) here?

same comment about which and using the ui constant

same comment about possibly replacing $('.branchesSelectionView') with this.$el

this.views[this._currentElement].$el.focus(); ended up working for me so we can treat that view as a jquery object but thanks for the suggestion!

Attempted that but then I couldn't key press back up to the branch dropdown itself. I think it needs an explicit element to focus back to so I left it as $('.branches-collection').focus(); instead.

Ah I didn't know that shift+tab was a thing haha thats a cool tidbit I learned today from you then :)
Now that's something I'll implement for sure not just for this view but for the others as well

This event will only occur if there is a keydown on the search field input itself specified by 'keydown .rb-c-search-field__input': '_onKeyDown',. Essentially this event happens when the element is in focus and the correct event occurs. The focus check happens implicitly I believe unless I'm interpreting this wrong. Could argue that this case would also apply to the other keydown handlers in the other files.

I left it has pascal cased as the other describe test suites suggest this

view.render() can be put in the beforeEach but not view.delegateEvents because the spies need to be setup first before this function is called. Essentially we want the spy wrapper callback to be called when the proper DOM event happens otherwise the tests will say the spy was not called.

mentioned using $('.branches-collection') in the other comment

This is great stuff, Monica. I loved seeing the demo.
Something that stood out is that, due to the layout, the focus indicator the browser uses isn't super helpful. It just appears as a line of focus, and it's not super clear which thing it's supposed to be indicating.
I think what we should do for the keyboard-based navigation is remove the focus outline (outline: 0) and instead set a background color (we can match the top bar's color maybe), so that it's very clear what item is currently the selected item.

Do we need to stop propagation/prevent default here?

Add a blank line between this block and the next

Please wrap to 80 columns.

Add a blank line above this one.

Please wrap to 80 columns.

Does this work even though view is never attached to the DOM?

I haven't actually tried running it, but this is looking really solid. Just a few style nits left.

This should be written in the imperative mood ("Handle a keydown event")

Imperative mood.

Imperative mood.

We don't need the inner parens on this first part.

Let's align all of this with the opening (. We also don't need the inner parens:

} else if (event.which === $.ui.keyCode.DOWN ||
           event.which === $.ui.keyCode.TAB) {

Imperative mood.

Imperative mood.

Imperative mood.