Show reason why live preview was cancelled #2178
Conversation
ghost
assigned
|
Hi Dennis--in a time crunch right now so we probably won't get to this till next sprint. Thanks! |
|
I just rebased this onto master, I hope Github reflects this soon... |
|
"The command "git checkout -qf cc564489a918a8d13a4a644dfb15f5be0b928ec5" failed and exited with 128 during checkout." Anything I can do? |
|
My Chrome is now at 25.0.1364.97 m and sends the dispatched event! Rebased the pull request onto master. |
|
Rebased |
|
Rebased once more - should another be necessary, I'll wait until prompted. |
|
Sorry I haven't gotten back to this. Will try to work on it this sprint. |
…since that's where the button is defined. Also hide the tooltip when connecting again.
|
Cool, thanks! Did another rebase anyway, to get rid of my unnecessary dealing with the Chinese translation + Windows git issues. |
|
Finally getting around to this. The error really helps, and I like the basic interaction. However, I ran into a problematic scenario: if the live preview browser overlaps the Brackets window (which is likely on a laptop, especially if you make it bigger in order to use the dev tools), and you spend a little time in the dev tools before switching back to Brackets, you'll never see the twipsy because it fades out before you switch back. We should make it so that if the Brackets window isn't active when the twipsy comes up, it doesn't fade out until after Brackets becomes active again. Could you look into making that change? |
|
Sounds reasonable, I'll look into it! |
Conflicts: src/LiveDevelopment/LiveDevelopment.js src/LiveDevelopment/main.js
… when the live preview has been cancelled in Chrome
|
@njx Done! Note that I put the window focus detection into twipsy-mod. I first wanted to add it to ViewUtils, but this would have made the jQuery module dependent on it. This way it seemed cleaner - until some other part of the system needs the same. Also I tried it via |
Conflicts: src/thirdparty/CodeMirror2
| @@ -301,6 +301,8 @@ define(function Inspector(require, exports, module) { | |||
| * -> Inspector.domain.command() | |||
| */ | |||
| function init(theConfig) { | |||
| var result = new $.Deferred(); | |||
There was a problem hiding this comment.
Thanks, overlooked this when refactoring
|
Detailed review done. The only substantive issue is the one about the error showing up when the user turns off Live Preview him/herself--we should fix that before merging. Otherwise, looks good. |
|
@njx: Thanks for the review! I think it's new behaviour that the popup also appears when the user closes live preview... could be a timing issue where we close the tab before we (would have) cut the connection ourselves. |
|
Hmmm. It appears that usually we are still connected after the window closes, so we do close the connection ourselves. Yet the popup appears. Maybe something changed on Chrome's end. |
|
Another thing that should be added is a popup if we close the connection ourselves because the user navigates away from the page. On the other hand we might rethink that; if I accidentally click a click and navigate back right away, I'd prefer live preview to still be active. |
Show a tooltip when live preview has been closes due to navigating away from the project
|
Hey Dennis--since it's my fault this has been sitting around so long, I went ahead and rebased it onto current master, and added a fix so the twipsy goes to the left of the icon since the toolbar's on the right now. I pushed that up in #3715, so closing this one. |
|
@njx: Wonderful, I'm sure I will smile when I unsuspectedly see the popup for the first time :) |
Fixes #1697