Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Add styles for stickyhighlight feature in livepreview #14546

Merged

Conversation

boopeshmahendran
Copy link
Contributor

No description provided.


function createWebSocket() {
_ws = new WebSocket("ws://localhost:" + remoteWSPort);
_ws.onopen = function () {
window.document.addEventListener("click", onDocumentClick);
if (!experimental && config.stickyHighlight) {
window.document.addEventListener("mouseover", onDocumentHover);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you would need to listen in the capture phase.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, when updateConfig is called, the event listener should be attached even at that time and disengaged when the config changes again.

@boopeshmahendran
Copy link
Contributor Author

@vickramdhawal Thanks for the quick review. I have addressed the review comments

Copy link
Collaborator

@swmitra swmitra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Border would potentially hide/override any Broder used in the actual UI. The ideal way to highlight is either by using a outline or drawing an overlay div by constructing an overlay pane on top of the entire page. Then use element from point of mouse event and use the targets bounding client rect to draw the highlight overlay. Usage of the overlay pane is required to prevent click on an actual link or button.

@boopeshmahendran
Copy link
Contributor Author

Thanks for the review @swmitra The approach you have described is the exact one followed as you can see here . The border is on the overlay div.

@swmitra
Copy link
Collaborator

swmitra commented Sep 25, 2018

But still not in an overlay pane. However use outline and the actual elements bounding box for positioning the highlight overlay and styling.

@vickramdhawal
Copy link
Collaborator

The current implementation follows the existing approach. Are you suggesting to introduce an overlay pane overall for both the highlight modes ?

@vickramdhawal vickramdhawal merged commit 3394f96 into vdhawal/livepreviewenhancements Sep 26, 2018
@boopeshmahendran boopeshmahendran deleted the boopeshmahendran/styling branch September 26, 2018 04:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants