-
Notifications
You must be signed in to change notification settings - Fork 7.5k
Added Reverse Inspect in Live Preview using WebSockets, and now forwa… #13044
Changes from 3 commits
7ac32b9
8c141ce
06b5418
71c4148
dbe1329
aae3943
819d122
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,15 +22,15 @@ | |
| */ | ||
|
|
||
| /*jslint forin: true */ | ||
| /*global Node */ | ||
| /*global Node, document */ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
| /*theseus instrument: false */ | ||
|
|
||
| /** | ||
| * RemoteFunctions define the functions to be executed in the browser. This | ||
| * modules should define a single function that returns an object of all | ||
| * exported functions. | ||
| */ | ||
| function RemoteFunctions(experimental) { | ||
| function RemoteFunctions(experimental, remoteWSPort) { | ||
| "use strict"; | ||
|
|
||
| var lastKeepAliveTime = Date.now(); | ||
|
|
@@ -98,6 +98,23 @@ function RemoteFunctions(experimental) { | |
| element.removeAttribute(key); | ||
| } | ||
| } | ||
|
|
||
| // Checks if the element is in Viewport in the client browser | ||
| function isInViewport(element) { | ||
| var rect = element.getBoundingClientRect(); | ||
| var html = document.documentElement; | ||
| return ( | ||
| rect.top >= 0 && | ||
| rect.left >= 0 && | ||
| rect.bottom <= (window.innerHeight || html.clientHeight) && | ||
| rect.right <= (window.innerWidth || html.clientWidth) | ||
| ); | ||
| } | ||
|
|
||
| // returns the distance from the top of the closest relatively positioned parent element | ||
| function getDocumentOffsetTop(element) { | ||
| return element.offsetTop + (element.offsetParent ? getDocumentOffsetTop(element.offsetParent) : 0); | ||
| } | ||
|
|
||
| // construct the info menu | ||
| function Menu(element) { | ||
|
|
@@ -319,6 +336,14 @@ function RemoteFunctions(experimental) { | |
| if (this.trigger) { | ||
| _trigger(element, "highlight", 1); | ||
| } | ||
|
|
||
| if (!window.event && !isInViewport(element)) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: not sure why the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am checking if there is any window event (mostly click event) is there on the LivePreview instance, then we don't need to bring the element in viewport, as it is already present in the viewport. |
||
| var top = getDocumentOffsetTop(element); | ||
| if (top) { | ||
| top -= (window.innerHeight / 2); | ||
| window.scrollTo(0, top); | ||
| } | ||
| } | ||
| this.elements.push(element); | ||
|
|
||
| this._makeHighlightDiv(element, doAnimation); | ||
|
|
@@ -824,7 +849,42 @@ function RemoteFunctions(experimental) { | |
| if (experimental) { | ||
| window.document.addEventListener("keydown", onKeyDown); | ||
| } | ||
|
|
||
| var _ws = null; | ||
|
|
||
| function onDocumentClick(event) { | ||
| var element = event.target, | ||
| currentDataId, | ||
| newDataId; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: newline here |
||
|
|
||
| if (_ws && element && element.hasAttribute('data-brackets-id')) { | ||
| _ws.send(JSON.stringify({ | ||
| type: "message", | ||
| message: element.getAttribute('data-brackets-id') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The message is just the element id, correct? When sending messages in other direction (from Brackets to browser), the message has the format: It would be nice to do the same here so that's it's easier to add any new messages in the future.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually in multi-browser also we are sending the message from browser to brackets in the same format(NodeSocketTransportRemote.js), that's why I also used the same format, so that in future when we implement the same thing for multi-browser, we can have the same message to interpret in the brackets side. |
||
| })); | ||
| } | ||
| } | ||
|
|
||
| window.document.addEventListener("click", onDocumentClick); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is better to move this call in the |
||
|
|
||
|
|
||
| function createWebSocket() { | ||
| _ws = new WebSocket("ws://localhost:" + remoteWSPort); | ||
| _ws.onopen = function () { | ||
| }; | ||
|
|
||
| _ws.onmessage = function (evt) { | ||
| }; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you don't use it, remove it? |
||
|
|
||
| _ws.onclose = function () { | ||
| // websocket is closed. | ||
| }; | ||
| } | ||
|
|
||
| if (remoteWSPort) { | ||
| createWebSocket(); | ||
| } | ||
|
|
||
| return { | ||
| "DOMEditHandler" : DOMEditHandler, | ||
| "keepAlive" : keepAlive, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,7 +94,9 @@ define(function LiveDevelopment(require, exports, module) { | |
| ProjectManager = require("project/ProjectManager"), | ||
| Strings = require("strings"), | ||
| StringUtils = require("utils/StringUtils"), | ||
| UserServer = require("LiveDevelopment/Servers/UserServer").UserServer; | ||
| UserServer = require("LiveDevelopment/Servers/UserServer").UserServer, | ||
| WebSocketTransport = require("LiveDevelopment/transports/WebSocketTransport"), | ||
| PreferencesManager = require("preferences/PreferencesManager"); | ||
|
|
||
| // Inspector | ||
| var Inspector = require("LiveDevelopment/Inspector/Inspector"); | ||
|
|
@@ -195,6 +197,10 @@ define(function LiveDevelopment(require, exports, module) { | |
| * Handles of registered servers | ||
| */ | ||
| var _regServers = []; | ||
|
|
||
| PreferencesManager.definePreference("livedev.wsPort", "number", 8125, { | ||
| description: Strings.DESCRIPTION_LIVEDEV_WEBSOCKET_PORT | ||
| }); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you are doing for LiveDevelopment, maybe you should do the same for MultiBrowserLivePreview too. See also #11957
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah once this is completed, I will try to do the same for multibrowser also. |
||
|
|
||
| function _isPromisePending(promise) { | ||
| return promise && promise.state() === "pending"; | ||
|
|
@@ -849,6 +855,7 @@ define(function LiveDevelopment(require, exports, module) { | |
| * @return {jQuery.Promise} Always return a resolved promise once the connection is closed | ||
| */ | ||
| function _close(doCloseWindow, reason) { | ||
| WebSocketTransport.closeWebSocketServer(); | ||
| if (_closeDeferred) { | ||
| return _closeDeferred; | ||
| } else { | ||
|
|
@@ -1362,6 +1369,7 @@ define(function LiveDevelopment(require, exports, module) { | |
| // wait for server (StaticServer, Base URL or file:) | ||
| prepareServerPromise | ||
| .done(function () { | ||
| WebSocketTransport.createWebSocketServer(PreferencesManager.get("livedev.wsPort")); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See @ficristo's comment above about the |
||
| _doLaunchAfterServerReady(doc); | ||
| }) | ||
| .fail(function () { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| /* | ||
| * Copyright (c) 2017 - present Adobe Systems Incorporated. All rights reserved. | ||
| * | ||
| * Permission is hereby granted, free of charge, to any person obtaining a | ||
| * copy of this software and associated documentation files (the "Software"), | ||
| * to deal in the Software without restriction, including without limitation | ||
| * the rights to use, copy, modify, merge, publish, distribute, sublicense, | ||
| * and/or sell copies of the Software, and to permit persons to whom the | ||
| * Software is furnished to do so, subject to the following conditions: | ||
| * | ||
| * The above copyright notice and this permission notice shall be included in | ||
| * all copies or substantial portions of the Software. | ||
| * | ||
| * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
| * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
| * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
| * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING | ||
| * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER | ||
| * DEALINGS IN THE SOFTWARE. | ||
| * | ||
| */ | ||
|
|
||
| /** | ||
| * This transport provides a WebSocket connection between Brackets and a live browser preview. | ||
| * This is just a thin wrapper around the Node extension (WebSocketTransportDomain) that actually | ||
| * provides the WebSocket server and handles the communication. We also rely on an injected script in | ||
| * the browser for the other end of the transport. | ||
| */ | ||
|
|
||
| define(function (require, exports, module) { | ||
| "use strict"; | ||
|
|
||
| var FileUtils = require("file/FileUtils"), | ||
| NodeDomain = require("utils/NodeDomain"), | ||
| EditorManager = require("editor/EditorManager"), | ||
| HTMLInstrumentation = require("language/HTMLInstrumentation"); | ||
|
|
||
| // The node extension that actually provides the WebSocket server. | ||
|
|
||
| var domainPath = FileUtils.getNativeBracketsDirectoryPath() + "/" + FileUtils.getNativeModuleDirectoryPath(module) + "/node/WebSocketTransportDomain"; | ||
|
|
||
| var WebSocketTransportDomain = new NodeDomain("webSocketTransport", domainPath); | ||
|
|
||
| // Events | ||
|
|
||
| WebSocketTransportDomain.on("message", function (obj, message) { | ||
| console.log("WebSocketTransport - event - message" + " - " + message); | ||
| var editor = EditorManager.getActiveEditor(), | ||
| position = HTMLInstrumentation.getPositionFromTagId(editor, parseInt(message, 10)); | ||
| if (position) { | ||
| editor.setCursorPos(position.line, position.ch, true); | ||
| } | ||
| }); | ||
|
|
||
| function createWebSocketServer(port) { | ||
| WebSocketTransportDomain.exec("start", parseInt(port, 10)); | ||
| } | ||
|
|
||
| function closeWebSocketServer() { | ||
| WebSocketTransportDomain.exec("close"); | ||
| } | ||
|
|
||
| exports.createWebSocketServer = createWebSocketServer; | ||
| exports.closeWebSocketServer = closeWebSocketServer; | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| /* | ||
| * Copyright (c) 2017 - present Adobe Systems Incorporated. All rights reserved. | ||
| * | ||
| * Permission is hereby granted, free of charge, to any person obtaining a | ||
| * copy of this software and associated documentation files (the "Software"), | ||
| * to deal in the Software without restriction, including without limitation | ||
| * the rights to use, copy, modify, merge, publish, distribute, sublicense, | ||
| * and/or sell copies of the Software, and to permit persons to whom the | ||
| * Software is furnished to do so, subject to the following conditions: | ||
| * | ||
| * The above copyright notice and this permission notice shall be included in | ||
| * all copies or substantial portions of the Software. | ||
| * | ||
| * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
| * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
| * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
| * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING | ||
| * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER | ||
| * DEALINGS IN THE SOFTWARE. | ||
| * | ||
| */ | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a JSDoc to describe the purpose of the file? |
||
| /*eslint-env node */ | ||
| /*jslint node: true */ | ||
| "use strict"; | ||
|
|
||
| var WebSocketServer = require("ws").Server; | ||
|
|
||
| /** | ||
| * @private | ||
| * The WebSocket server we listen for incoming connections on. | ||
| * @type {?WebSocketServer} | ||
| */ | ||
| var _wsServer; | ||
|
|
||
| /** | ||
| * @private | ||
| * The Brackets domain manager for registering node extensions. | ||
| * @type {?DomainManager} | ||
| */ | ||
| var _domainManager; | ||
|
|
||
| /** | ||
| * @private | ||
| * Creates the WebSocketServer and handles incoming connections. | ||
| */ | ||
| function _createServer(socketPort) { | ||
| if (!_wsServer) { | ||
| // TODO: make port configurable, or use random port | ||
| _wsServer = new WebSocketServer({port: socketPort}); | ||
| _wsServer.on("connection", function (ws) { | ||
| ws.on("message", function (msg) { | ||
| console.log("WebSocketServer - received - " + msg); | ||
| var msgObj; | ||
| try { | ||
| msgObj = JSON.parse(msg); | ||
| } catch (e) { | ||
| console.error("webSocketTransport: Error parsing message: " + msg); | ||
| return; | ||
| } | ||
|
|
||
| if (msgObj.type === "message") { | ||
| _domainManager.emitEvent("webSocketTransport", "message", msgObj.message); | ||
| } else { | ||
| console.error("webSocketTransport: Got bad socket message type: " + msg); | ||
| } | ||
| }).on("error", function (e) { | ||
| console.error("webSocketTransport: Error on socket : " + e); | ||
| }).on("close", function () { | ||
| console.log("webSocketTransport closed"); | ||
| }); | ||
| }).on("error", function (e) { | ||
| console.error("webSocketTransport: Error on live preview server creation: " + e); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Initializes the socket server. | ||
| * @param {number} port | ||
| */ | ||
| function _cmdStart(port) { | ||
| _createServer(port); | ||
| } | ||
|
|
||
| /** | ||
| * Kill the WebSocketServer | ||
| */ | ||
| function _cmdClose() { | ||
| if (_wsServer) { | ||
| _wsServer.close(); | ||
| _wsServer = null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Initializes the domain and registers commands. | ||
| * @param {DomainManager} domainManager The DomainManager for the server | ||
| */ | ||
| function init(domainManager) { | ||
| _domainManager = domainManager; | ||
| if (!domainManager.hasDomain("webSocketTransport")) { | ||
| domainManager.registerDomain("webSocketTransport", {major: 0, minor: 1}); | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Newline here |
||
|
|
||
| domainManager.registerEvent( | ||
| "webSocketTransport", | ||
| "message", | ||
| [ | ||
| { | ||
| name: "msg", | ||
| type: "string", | ||
| description: "JSON message from client page" | ||
| } | ||
| ] | ||
| ); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Newline here |
||
|
|
||
| domainManager.registerCommand( | ||
| "webSocketTransport", // domain name | ||
| "start", // command name | ||
| _cmdStart, // command handler function | ||
| false, // this command is synchronous in Node | ||
| "Creates the WS server", | ||
| [ | ||
| { | ||
| name: "port", | ||
| type: "number", | ||
| description: "Port on which server needs to listen" | ||
| } | ||
| ], | ||
| [] | ||
| ); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIt: newline here |
||
|
|
||
| domainManager.registerCommand( | ||
| "webSocketTransport", // domain name | ||
| "close", // command name | ||
| _cmdClose, // command handler function | ||
| false, // this command is synchronous in Node | ||
| "Kills the websocket server", | ||
| [] | ||
| ); | ||
| } | ||
|
|
||
| exports.init = init; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "name": "brackets-livedev-server", | ||
| "dependencies": { | ||
| "ws": "~0.4.31" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can put it on the root package.json and update the shrinkwrap file.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, right now we have to run |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#12949 will change this to pass a config object, maybe you can already change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have run all tests related to Live Preview, and now MultiBrowserPreview is also working fine.
Regarding passing of config object, once #12949 is merged, I will do the respective changes.