fiddle
fiddle copied to clipboard
electron
fix: tab focus for settings page
Hello there,
Thought I'd try look into the issue (#1083) with tab focus misbehaving when the settings page is open.
I can confirm those tooltips come about because tabbing through the settings page eventually runs out of elements and goes on to tab through elements that are part of either the header or editor area.
The least invasive solution I could think of is to ensure the header and editor area elements are made invisible. display: none is a bit more destructive than visibility: hidden, because the former may interfere with components not rendering at all and potentially losing their state.
Now, I'm not awfully familiar with MobX, but I tried my best to work in some changes that do what I described above.
Happy to take feedback and revise (or add a test, which I couldn't work out how to do for this) as necessary. 🙂
As an aside, I was curious why tabbing inside those overlay dialogs worked better, so I also looked there at what's going on. It seems to be that the underlying Blueprint dialog implementations have some special code which ensure tabbing only cycles through within the overlay.
Coverage decreased (-0.1%) to 91.395% when pulling 20432c2cde836edfb2ad2a3ef7354dcc3c8e7d4b on miqh:fix/settings-tab-focus into 8cbc9c998153d512da28413204a3d5618aa36dd7 on electron:main.
@erickzhao, funny you should ask that...
I actually started out making changes in app.tsx to apply the CSS classes there because it's obviously a more central choke point. However, I wasn't able to figure out how to do it.
I think something needs to be done with that plain old JSX element being passed into ReactDOM.render() to make it aware and observing the isSettingsShowing property part of application state. My MobX-fu and understanding isn't up to scratch on how best to do this I'm afraid. All I know is that it's probably not wise to be conditionally rendering Header or OutputEditorsWrapper for fear that might do strange things to application state.
If you can point me in the right direction I can try have another stab, otherwise I'll just close these changes.
