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:

[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]

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.