-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Fix 8131: installing extensions with post-install dialogs causes brackets to hang #8210
Conversation
… when we can't focus a button
@dangoor do you have time to review this tonight? I can do it if you can't. |
We should do some testing to make sure CodeMirror reacts ok to this... sometimes it can be finicky about the manner in which it loses focus. (I think this will be ok but it just deserves some extra testing attention, is all). |
hmmm... so I tested this and it does, indeed prevent the endless focus loop. However, it also disables the dialogs that Theseus and Bracket-Git used to pop up after installation. I don't know if that is intended or not. I tend to agree with @dangoor that popping modal dialogs after install is not good UI (what if I install more than 1 extension per extension manager session? this is bound to cause problems so the extensions should not do this imo) so perhaps disabling them is a good thing. |
@mackenza What do you mean by "disables the dialogs that Theseus and Bracket-Git used to pop up after installation"? |
I just did a quick test of the sort I did earlier to bisect (deleted the brackets-git entries in state.json and removed brackets git, then restarted and reinstalled brackets-git). For me, the brackets-git dialog appeared just fine over top of the extension manager. @JeffryBooher Though this is only a 2 line change, they're two lines I'm not really familiar with. I'm too tired to review this tonight. |
@TomMalbran I meant that the post install/first use dialogs didn't appear for me. I will test again as what I am seeing seems different from @dangoor |
@mackenza It should appear. But for Brackets-Git I think that you need to delete the state.json file or at least delete the entries for brackets-git, as @dangoor mentioned. @dangoor Is 2 lines modified, but one was the cause of the error, so it might be good to test it. The tabindex attribute, was so that the dialog could gain focus, which was used so that we could focus it in case no buttons were available in the dialog. Since I removed that, I needed a way to blur what was in focus without focusing the dialog, so that second line does that. It blurs the element with focus. |
I tested this and the brackets-git and theseus extension post-install dialogs are displayed. I deleted the extensions and all of my state and cached cef data and installed them (this is how most users would experience it after installation) and it seems to work fine. It prevents the endless focus pong and doesn't introduce any new focus issues to the app. I'm going to merge this into master. This will give us a few days to play with it. |
Fix 8131: installing extensions with post-install dialogs causes brackets to hang
When fixing #7598 I gave the focus to the dialog when it had no inputs. But to do it I had to make the dialog focusable, which makes Bootstrap enter into an infinite focus loop. Since the idea about giving focus to the dialog was to blur current editor, this alternative fix, actually blurs any element that had focus.
This fixes #8131 and other similar issues.