-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Ignore Live Dev toggle when in a connecting state #7049
Conversation
Unfortunately, this is not fixing the issue. It's a great improvement and I need to click far more often too before it stops working. But then I'll see the "Unable to load Live Development page" and the "Needs to restart Chrome" Error. |
I'm not sure exactly how we'd handle the "click on button as many times as you can and as fast as you can" case -- I think this applies to almost any button in brackets. I think this is helpful for the case where Live Preview is connecting very slowly or connection is timing out and user is wondering what's going on (so they click again). Do you think it's worth getting this change in for Release 37, and then leave issue open to try to improve more? |
I've also seen some people double-click the icon right off the bat, which seems like it could cause issues too. |
@redmunds I think this is already an improvement over the previous behavior and I'm fine with merging this PR and iterate on this, if we receive more reports that users hit this issue. My behavior of constantly clicking the live preview icon was probably not even close to what a user might be doing, but it's not very unlikely too. For now, we should merge this PR and wait for feedback. @peterflynn what do you think? |
@ingorichter I updated this branch for the new Chrome v34 API changes that I merged into master last night. @peterflynn This change handles double-click when opening Live Preview. But closing Live Preview is much faster, so it's already INACTIVE by the time the second click arrives, so Brackets is re-opened. Note: with Chrome v34, you will see the following error, but only once:
This is not really an error, so maybe this message should be suppressed? @jasonsanjose Is there a way to detect what version of Chrome is being used so we could suppress this if version >= 34? |
@redmunds I thought there would be some way to ask the inspector protocol what version is running, but there isn't. We'd have to modify our startup to do some user agent sniffing and make a best guess from there on whether or not to try the deleted API. |
@redmunds I'm going to give it a try later this afternoon. |
@jasonsanjose Any way to make this message sound less like an error?
Maybe something like:
|
@ingorichter I branched off this PR into https://github.com/adobe/brackets/compare/jasonsanjose;issue-7014?expand=1. I added an extra precautions around reentrancy into |
Are we planning to merge this for Sprint 37? |
I would like to merge this. I'm currently testing it. |
I would like to yes. I'm working with @ingorichter and @redmunds to see if my branch (with Randy's fixes) fixes the additional issues Ingo was seeing. |
OK, sounds good. |
Tagged sprint 37 |
@jasonsanjose this looks much better now. I haven't been able to repro the dialogs. I've tested your branch with Chrome 34 Beta and Chrome 33 Stable. |
Thanks @ingorichter. I'll close this PR and open a new one for my branch, see #7148. |
This is for #7014
cc @ingorichter