WIP: Feature: Add setting to ignore components

[KevinBatdorf]

This also includes wiring up settings management in general, using the ignore setting as the example case.

TODO:

1. Add option with text field, perhaps just run whatever they enter though document.querySelectorAll() with a default of x-devtools-ignore
2. Set up storage sync flow with onChange event to auto update devtools on change.

https://developer.chrome.com/docs/extensions/reference/storage/

I'll share thoughts on the UI a bit more after I add an option and description, so we can discuss things early on.

But currently I tweaked things a bit to look like this:

[ryangjchandler]

Just some spelling errors

[KevinBatdorf]

Thanks. I think you can suggest the change directly in the review then I can just "accept it"

Had to change a filename though anyway.

[KevinBatdorf]

Thanks. I think you can suggest the change directly in the review then I can just "accept it"

I thought this was the case but trying to do this on the other PR and it doesn't seem to work as it did before. hmm

edit: Ah you press the "Add suggestion" icon

[KevinBatdorf]

Added an input now. it doesn't do anything yet. I'll work on wiring it all up tomorrow.

Open to feedback and suggestions.

[KevinBatdorf]

Thinking this will only work on the simulator. Will see what i can do. It's otherwise duplicating the settings page.

[HugoDF]

How about adding data-testids to the panel "root" components, data-testid="panel-root" maybe?

Would have to be added to both

alpinejs-devtools/packages/shell-chrome/views/panel.edge

Line 16 in 3b776fc

<div x-data x-show="false" class="preload">Devtools loading...</div>

and

alpinejs-devtools/packages/shell-chrome/views/panel.edge

Line 18 in 3b776fc

<div id="app" class="h-full" x-cloak x-data="devtools()" x-init="init()" x-spread="devtoolsRootDirectives()">

Should keep the selectors simpler and it matches our use-case (we're selecting using the testid's for dev/testing purposes only)

[KevinBatdorf]

Updated now if you want to check it

[HugoDF]

Looks good to me

[KevinBatdorf]

@HugoDF It doesn't seem like this is hurting anything, but is there something I'm maybe missing?

[HugoDF]

there isn't but it might work out better to use testid's on the "root panel components" we want to inject in the simulator (see #136 (comment))

[KevinBatdorf]

Just realized the map is only run in the simulator anyway, so it's not necessary here regardless.

[KevinBatdorf]

Accessing storage seems quite involved so probably worth chatting about it. Not entirely sure how to set up the communication from background.js to the devtools without some big refactoring or just adding a separate channel

I think it needs to be separate from the existing proxy that is set up, right?

Essentially, I think we want to do this:

// background.js
chrome.storage.sync.get(['alpine-devtools-settings'], (result) => {
  // Load alpine in here so the initial settings can be passed in
})
chrome.storage.onChanged.addListener((changes, namespace) => {
  // not sure how this works yet, but it should push an update to devtools when a setting is changed elsewhere
})
function updateSettings(settings) {
  chrome.storage.sync.set({'alpine-devtools-settings': settings}, () => {
    // report back OK or not
  })
})

// devtools.js
// Should have a listener for the sync update and a function to talk back to background.js

edit: Looks like you added something like I was thinking about on the warnings PR, right? Should we maybe create a generic API with a handler function?
https://github.com/alpine-collective/alpinejs-devtools/pull/140/files

[HugoDF]

@KevinBatdorf how about using

https://www.github.com/alpine-collective/alpinejs-devtools/tree/master/packages%2Fshell-chrome%2Fsrc%2Fdevtools-background.js

Does that have access to storage? Since it's creating the panel I'm thinking it might be able to send it some data

[HugoDF]

Otherwise in https://www.github.com/alpine-collective/alpinejs-devtools/tree/master/packages%2Fshell-chrome%2Fsrc%2Fbackground.js

You've got access to all the "tabs" (using Object.values(ports)) so you could loop through the tabs and send a message to all devtools instances

[KevinBatdorf]

@HugoDF I looked into this finally and it's actually an issue with the simulator. The sync settings work fine using the actual devtools.

So what that means is we can't test anything and it's slightly more annoying to develop. I'm thinking maybe we just hold off on having a settings page for now and focus on other improvements?

[HugoDF]

@HugoDF I looked into this finally and it's actually an issue with the simulator. The sync settings work fine using the actual devtools.

So what that means is we can't test anything and it's slightly more annoying to develop. I'm thinking maybe we just hold off on having a settings page for now and focus on other improvements?

That kind of makes sense, I think it's fine to have the settings page untested (unless there's an easy way to fake it)

[KevinBatdorf]

Yeah we can stub out some defaults for basic testing and show an error message. The error message we're getting is "Error: Cannot read property 'sync' of undefined" from chrome.storage.sync.get

[HugoDF]

Yeah we can stub out some defaults for basic testing and show an error message. The error message we're getting is "Error: Cannot read property 'sync' of undefined" from chrome.storage.sync.get

Yeah let's do that to get existing tests passing. You said it's all working just not in the simulator? That's fine

[KevinBatdorf]

You said it's all working just not in the simulator?

chrome.storage.sync.get is working outside the simulator, yeah. This PR still needs work though.

Another blocker is that you can't dispatch events like you do in the browser. I haven't wrapped my head around how the communication works from front -> back -> front, etc either. Not sure the best way to communicate between components otherwise, so now the settings component will just re-try until the data is available. It's probably ok.

I'll try to work on it some more tomorrow.

[KevinBatdorf]

Status update. So the sync works and settings are saving properly. The issue I'm facing now is that we need to be able to access the settings from the backend.js before the front devtools renders, and I'm not entirely clear on how the back and front communicate to each other.

It needs to happen right before this line:

https://github.com/alpine-collective/alpinejs-devtools/blob/master/packages/shell-chrome/src/backend.js#L108

That way we can filter out what gets sent to the devtools.

@HugoDF You probably know best how to post a message to the front, get the settings, and send them to the back?

[HugoDF]

🤔 so the way to do that is either to add it to the init message payload (which is handled in handshake), not sure where it originates from though

If that's too early/we dont have the data from storage at that point, we can send a fresh message when we do have the data, that'll trigger the component discovery & has the preferences

(I haven't pulled this down and given it a go, when I do I can be more specific 🙂 or even PR something into this one)

[KevinBatdorf]

I'll try that out and see. But feel free to take over this PR and work on it if you have an idea. Just send me a DM if you will so we're not both working on changes.