Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Fix 8131: installing extensions with post-install dialogs causes brackets to hang #8210

Merged
merged 1 commit into from
Jun 21, 2014

Conversation

TomMalbran
Copy link
Contributor

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.

@JeffryBooher
Copy link
Contributor

@dangoor do you have time to review this tonight? I can do it if you can't.

@peterflynn
Copy link
Member

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).

@mackenza
Copy link
Contributor

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.

@TomMalbran
Copy link
Contributor Author

@mackenza What do you mean by "disables the dialogs that Theseus and Bracket-Git used to pop up after installation"?

@dangoor
Copy link
Contributor

dangoor commented Jun 21, 2014

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.

@mackenza
Copy link
Contributor

@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

@TomMalbran
Copy link
Contributor Author

@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.

@JeffryBooher JeffryBooher added this to the Release 0.41 milestone Jun 21, 2014
@JeffryBooher
Copy link
Contributor

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.

JeffryBooher added a commit that referenced this pull request Jun 21, 2014
Fix 8131: installing extensions with post-install dialogs causes brackets to hang
@JeffryBooher JeffryBooher merged commit 7faa40d into master Jun 21, 2014
@JeffryBooher JeffryBooher deleted the tom/dialog-focus-fix branch June 21, 2014 23:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

installing extensions with post-install dialogs causes brackets to hang
5 participants