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.

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

[CLOSED] Added Reverse Inspect in Live Preview using WebSockets, and now forwa… #11085

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

Comments

@core-ai-bot
Copy link
Member

Issue by saurabh95
Tuesday Jan 17, 2017 at 04:39 GMT
Originally opened as adobe/brackets#13044


Now you can click on any element in Live Preview and then corresponding HTML element will be highlighted and the cursor in code view will shift to the corresponding HTML element.

Features that need to be incorporated in this PR

  • Get Socket Port Number from preferences.
  • Add commands to create and destroy server and create server should be called only when user clicks on Live Preview button.

I am currently working on these. Will update the PR soon.


saurabh95 included the following code: https://github.com/adobe/brackets/pull/13044/commits

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Tuesday Jan 17, 2017 at 10:38 GMT


I'm not really familiar with LivePreview to judge the approach. However I left some comments.

@core-ai-bot
Copy link
Member Author

Comment by petetnt
Tuesday Jan 17, 2017 at 12:19 GMT


Left some small comments too, looking good so far 👍

@core-ai-bot
Copy link
Member Author

Comment by saurabh95
Wednesday Jan 18, 2017 at 08:33 GMT


I have fixed some issues related to MultiBrowserLivePreview as RemoteFunctions.js was also included in MultiBrowserLivePreview. I have run the tests also, on both Firefox and Chrome.

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Thursday Jan 19, 2017 at 12:41 GMT


There are still a couple of unanswered comments.
Also in HTMLInstrumentation.js can you use lodash instead of the for.
_.find seems what you need.
Finally, you should check the tests and maybe add one?

@core-ai-bot
Copy link
Member Author

Comment by saurabh95
Saturday Jan 21, 2017 at 18:01 GMT


@ficristo, I just uploaded new changes, regarding the testing part, I was thinking of writing tests once I am done with MultiBrowserLivePreview as well(I am almost done with it), then I will write tests for both of them.
I have run the current LivePreview tests, and all are passing.

@petetnt@zaggino@MarcelGerber@abose Please review

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Tuesday Jan 24, 2017 at 14:46 GMT


This does not work on distribution.
To verify

  • run grunt build
  • open Brackets with the Shift keyword pressed
  • select the index.html under dist/ folder

You have to adjust the copy task in Gruntfile.js

@core-ai-bot
Copy link
Member Author

Comment by saurabh95
Wednesday Jan 25, 2017 at 07:17 GMT


Thanks@ficristo for pointing out. I have fixed this and also updated the PR :)

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Wednesday Jan 25, 2017 at 10:42 GMT


I looked enough at this so approving but I see some failures on Live Preview tests, so please take a look.

@core-ai-bot
Copy link
Member Author

Comment by saurabh95
Wednesday Jan 25, 2017 at 13:58 GMT


@ficristo Actually there is some problem with the timeout of the Live Preview tests, I increased the timeout and all the Live Preview tests are passing.

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Friday Feb 10, 2017 at 13:34 GMT


@saurabh95 the increase of timeout should be part of this PR?

@core-ai-bot
Copy link
Member Author

Comment by saurabh95
Friday Feb 10, 2017 at 14:53 GMT


@ficristo Actually without my changes also some LivePreview tests(the same ones) were failing and on increasing timeout they were passing, so I thought we can take change it in some other set of changes, as it is independent of this change?
Anyway I will be taking up the task to write tests for my changes, we can then incorporate the changes related to timeout there itself.

@core-ai-bot
Copy link
Member Author

Comment by swmitra
Tuesday Feb 14, 2017 at 05:51 GMT


Great work@saurabh95. This is indeed a great addition, keep more coming 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant