Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CLOSED] Live Preview MultiBrowser improvements (#10206, #10239) #9112

Open
core-ai-bot opened this issue Aug 30, 2021 · 12 comments
Open

[CLOSED] Live Preview MultiBrowser improvements (#10206, #10239) #9112

core-ai-bot opened this issue Aug 30, 2021 · 12 comments

Comments

@core-ai-bot
Copy link
Member

Issue by busykai
Saturday Dec 27, 2014 at 22:04 GMT
Originally opened as adobe/brackets#10285

Fix #10206 (point 1):

PreferencesManager.on("change") was executing before AppInit.appReady().
This was causing various inconsistencies, including setting up the UI before
LiveDevelopment is actually initialized. Handling of pref change is now moved
to AppInit.appReady and the implementation is set only after the modules are
actually initialized.

Fix #10239:

Toggle menu item to enable/disable multi-browser live preview. Under File -> Enable Experimental Live Preview. Multiple fixes to implementation life-cycle to support correct state transitions on switch.

Minor nits included: pref definition improved, suite name made shorter, spare lines removed, documentation improvements.

CC:@peterflynn,@sebaslv,@redmunds

busykai included the following code: https://github.com/adobe/brackets/pull/10285/commits
@core-ai-bot
Copy link
Member Author

Comment by busykai
Friday Jan 09, 2015 at 22:10 GMT

@redmunds, this is ready for a review.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Saturday Jan 10, 2015 at 00:44 GMT

@busykai I found a weird case when testing this. Let me know if I should file this:

  1. Start Bracket s on a system with a local server setup and running
  2. Switch to a site with files on the local server
  3. I originally hit this by setting base url to be http://localhost/, but this is not required
  4. Switch to LiveDev MultiBrowser
  5. Start Live Dev

Results
Live Dev never completely connects. It stays in Connecting state.

I don't think it's related to this change. Let me know.

@core-ai-bot
Copy link
Member Author

Comment by busykai
Monday Jan 12, 2015 at 17:19 GMT

@redmunds, thanks for the use case! we will take a look.

CC@sebaslv

@core-ai-bot
Copy link
Member Author

Comment by sebaslv
Wednesday Jan 14, 2015 at 15:16 GMT

@redmunds, I'm trying to reproduce this case but still couldn't do it. This is what I'm doing:

  1. Start a local server which serves let say index.html file
  2. Start Brackets
  3. Open the folder that contains the index.html being served
  4. Open http://localhost/index.html in a browser
  5. File -> Enable Experimental Live Preview
  6. Start Live Dev

By following these steps I got LiveDev working. Please, let me know if those are representative of the steps you have described or if I misunderstood something. Thanks.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Wednesday Jan 14, 2015 at 15:58 GMT

@sebaslv I did not open in browser first (step 4), so not sure if that's making a difference.

FYI, I'm seeing this on Win7 Firefox (latest) with WampServer 2.2 .

@core-ai-bot
Copy link
Member Author

Comment by sebaslv
Wednesday Jan 14, 2015 at 18:57 GMT

@redmunds, Thanks for the info. I was able to reproduce it now by configuring local server, the step I missed before I think :).

It seems that it never connects beacuse the page is not instrumented so the connect message never comes back from the browser after launching it. It looks like in this scenario, LiveDevMultiBrowser is getting an instance of UserServer since it has been registered by LiveDevelopment and both implementations share LiveDevServerManager.

Currently we're not managing that error. I'll create a fix for that independently of this case since it's pending. The thing is that it would close the connection which is something different to what the current implementation does in this scenario. I see that it launches the page and moves the icon to a connected status but keep live editing disabled. I could figure out how to identify the case and handle that in this particular way but not sure if that is the right UX we would like for this.

What do you guys suggest?

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Wednesday Jan 14, 2015 at 19:19 GMT

@sebaslv I agree that it should be a separate issue and not hold up this PR. I'll open new issue.

Seems like doing a better job of cleaning up when changing implementations should fix that.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Wednesday Jan 14, 2015 at 19:26 GMT

@sebaslv I filed issue #10374 . I'm not sure I understand all of the subtle points, so feel free to update it.

@core-ai-bot
Copy link
Member Author

Comment by sebaslv
Wednesday Jan 14, 2015 at 19:41 GMT

Sounds good. I'll create a PR for that issue. Thanks.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Wednesday Jan 14, 2015 at 22:24 GMT

@busykai Done with review. This looks good. Just need to backout changes to DebugCommands/main.js and squash commits.

@core-ai-bot
Copy link
Member Author

Comment by busykai
Thursday Jan 15, 2015 at 06:12 GMT

@redmunds, done!

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Jan 15, 2015 at 18:13 GMT

Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@core-ai-bot