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

Devtools cleanup #145

Open
wants to merge 57 commits into
base: dev
Choose a base branch
from
Open

Devtools cleanup #145

wants to merge 57 commits into from

Conversation

dkrasner
Copy link
Contributor

Motivation and Context

This PR represent a refactoring and cleanup of the devtools extension. Previous there were a number of drawbacks:

  1. The devtools panel <-> background <-> content script port communication API was convoluted. I found it confusing and hard to debug each time.
  2. using inspectedWindow.eval() for devtools panel -> cells was awkward, hard to debug and potentially dangerous
  3. handling of messages from, and sending data to, devtools was using a combination of ports, window message api and eval()

Approach

Ports are not cleanly defined in both background and content scripts.

Wherever possible the content script handles messages itself, ie it has its own set of utils which do not depend on the cells themselves. For example, mouseover type highlighting for cells only requires the DOM document query API and should be handled by CellHandler as before. Only in the case that something from Cells itself is strictly necessary (for example getting a cell nodes source code) is the window message API used.

This PR can close previous #139 and #140

How Has This Been Tested?

In devtools. The Tree lib has its own tests.

Screenshots or console output (if appropriate)

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

dkrasner added 30 commits October 1, 2022 19:14
@dkrasner
initial devtools extension setupContextMenu
4cd4df9 
manifest, devtools html/js, cells_panel html/js
@dkrasner
adding d3 tree viewer to the devtools cells panel
f0a6e78 
(currently with fake data)
@dkrasner
initial structuring of the d3 code into something reasonable
a4333ee
@dkrasner
adding info split-panel for cells
d11bf67 
organizing code into module
updating some css
@dkrasner
adding background and content-script js files
93cf81b
@dkrasner
updating communication between background target and devtools
9eb8459 
issues with Manfiest v3 TODO
@dkrasner
adding a README
019b704 
with png
and minor changes
@dkrasner
adding handleMessageFromBackground to devtools display
259b75d 
initial load, reconnection displays
setup for cells loadeded displays
@dkrasner
first pass at getting tree data to debugger
2023e27
@dkrasner
minor update to tree building for devtools
233bf51
@dkrasner
wrapping up devtools cell tree data generator and msg method
0565251
@dkrasner
adding mouseover and leave for cell tree nodes
a23ff93 
fixing up id class for d3 nodes and cells
@dkrasner
updaing mouseover and leave
ecc9d52
@dkrasner
updating devtools readme
cc0608c
@dkrasner
adding a better check (on the devtools side) to see if tree updated
2ec08b7
@dkrasner
limiting the node name length in tree
f830156
@dkrasner
template setup for tree component
70969b2
@dkrasner
adding TreeNode and updating basic example setup
8f8fcf3
@dkrasner
adding depths to trees and webpack
7fbf00c
@dkrasner
[WIP] rewriting Tree as regular object to dealing with leaderlines
947d46f
@dkrasner
[WIP] add tree.css and minor updates to leaderlines
44676d4
@dkrasner
v0 of working tree with conneiton lines
de7677b
@dkrasner
leadrlines use proper js document.createElementNS API
a94d0cf 
adding tests for the tree component
@dkrasner
adding global css, better styling and window resize listener
64175ad
@dkrasner
adding subtree zoom in/out and related css updates to tree and node
6f75f80 
adding a readme
@dkrasner
optimizing svg line rendering; adding findParentSubTree for nav
1344b95
@dkrasner
adding keyboard events for better scrolling up tree
993661d
@dkrasner
initial integration of tree [WIP]
9cd87ac
@dkrasner
adding proper display depth and related
b872dd8 
first pass at bezier curves for connectors
@dkrasner
fixing up connector lines in tree
0675f74
@dkrasner
adding handling of DOM invalid cell/node ids
06620c1 
prepending "node-" to each DOM element id
keeping the original id in "data-original-id" attribute
@dkrasner
addiung non-starting-root node handling
17c8426 
fixing up navigation bugs
@dkrasner
fixing up vertical paths in tree and adding on hover highlight
098b492 
when tree nodes are (almost) vertically aligned, use a basic svg path line
instead of a bezier curve
adding hover (mouseover mouseleave) handling on the target window object and
corresponding no-op methods to tree
setup these methods
@dkrasner
adding forgotten setupDevtools() in CellHandler
1d35ea0
@dkrasner
typo fix
580767f
@dkrasner
hightlight query selector fix
9006db8
@dkrasner
adding better overlay cleanup for devtools
6db1ea7
@dkrasner
adding better hover devtools highlighting and related changes
35b3cb1
@dkrasner
filtering out non cells from messager to devtools in CellHandler
d8871ce
@dkrasner
removing old d3 tree js file
f04608c
@dkrasner
adding node click and customization handlers
9b0227b 
click handler display node data in the info panel
customization performs additional highlighting
@dkrasner
improving the bezier curves connecting trees
4de7667
@dkrasner
updating cell info panel
a6ee429
@dkrasner
temp removal of child node display in info panel
71d0049 
looks like a weird race condition
@dkrasner
minor clean and improvement of cells panel
6de8bd8
@dkrasner
adding devtools request for cells source code
80274df
@dkrasner
adding print statement to cell
29f0330
@dkrasner
pausing cells messages
3a80632
@dkrasner
setting up target window call to get cell source
f631598
@dkrasner
fixing up cell lookup
7e5b8a5
@dkrasner
adding a quick install and a readme
bf7b43c
@dkrasner
testing setting panel to background communication in devtools
74f6b8f
@dkrasner
cleaning up ports in devtools
931cd27 
cleaner setup for content-script <-> background
background <-> panel/devtools window port API
@dkrasner
refactoring inspected window handling CS message handling
15da70b 
moving forward the background will not use the direct inspected window
eval() - which is a) awkward and b) can have security issues. All communication
from the cells panel routes via the background, to the content script which then
handles messages as needed (either directly executing utils or sending messages
via the window message api)
@dkrasner
setting up cell source code request via the window message api
3f18df3
@dkrasner
typo correction
1dff900
@dkrasner dkrasner requested a review from braxtonmckee June 19, 2023 12:10
This was referenced Jun 19, 2023
Closed
Closed
@dkrasner dkrasner mentioned this pull request Aug 7, 2023
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@braxtonmckee braxtonmckee Awaiting requested review from braxtonmckee

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@dkrasner