Focus the viewpane on the next tick

[lionel-]

Follow-up to #2867
Addresses another aspect of #811
Closes #2904

Intent

Prevent panel toggling from breaking viewpane toggling when there is no focusable element in the pane.

Approach

In #2867 we made viewpanes focusable by giving them a tabIndex. The ViewPane::focus() method (called via super.focus()) is in charge of focusing that pane. This allows focus() to succeed, which is necessary for viewpane toggling to work as expected.

However when you toggle the whole panel (e.g. with Cmd-J or the icon in the top-right), as opposed to toggling specifically the console viewpane, our focus() method is called too early before the pane is mounted to the DOM. This causes the focus call to fails, which then breaks viewpane toggling. To work around that I put our input focus call in a timeout. But because I didn't do the same with super.focus(), we can still get in a broken state by toggling the whole pane when there is no input to focus, which can happen during runtime discovery or when a runtime has crashed during startup. This PR fixes that omission.

I also added the comment that you requested in the other PR @nstrayer.

QA Notes

One way of reproducing is to disable automatic startup of interpreters. Without starting an interpreter, use the viewpane toggling command:

Do this a few times and you should see the console toggle.

Now use the panel toggling command, e.g. Cmd-J by default on mac (or the secondary side bar if the console is there). Do this a few times, it should work. However, before the fix, the console toggling command would now be broken. With the fix you should still be able to use it.

[lionel-]

Of course the problem happens with other panes like Variables. So I've gone ahead and implemented the PositronViewPane class to make sure all viewpanes benefit from these fixes.

This class sets tabIndex and overrides focus() to call it at the next tick. It also calls focusElement() if implemented by the subclass. This way we first focus the viewpane as a whole, which is our default, then an element inside it if possible.

I'm not sure why we need to focus at the next tick when VS Code panes don't. It feels like I'm missing something, but at least this is now working properly.

[nstrayer]

Code looks good to me. I would imagine the reason we need a timeout but vscode doesn't is that react may not render perfectly on the next tick. E.g. there may be a set state or something that causes a redraw of the dom that the react dom-diff algorithm doesn't know how to handle with focus. Also, it looks like a whole bunch of overrides are done for ViewPane.focus(), so it doesn't seem like a real code-smell.

[nstrayer]

of course this exists in async.ts! Good find!