quoid / userscripts

An open-source userscript manager for Safari
https://quoid.github.io/userscripts/
GNU General Public License v3.0
3.02k stars 155 forks source link

Feature request: GM.registerMenuCommand #230

Open iFelix18 opened 2 years ago

iFelix18 commented 2 years ago

API to register a command to be displayed in the Userscripts pop-up menu.

Reference: Greasemonkey, Violentmonkey, and Tampermonkey.

quoid commented 2 years ago

Thanks for the feature request @iFelix18 - I'll consider this, but there's a long list of updates already in queue.

For the time being you might want to consider @run-at content-menu - I don't think this is documented in the readme, but that will add a new context menu (right click) item. Sometimes you need to right click twice to get it show.

Obviously, this is desktop only.

// ==UserScript==
// @name        Context Test
// @description This is your new file, start writing code
// @include        *
// @run-at context-menu
// @inject-into auto
// ==/UserScript==

alert("context test 1");
m-thomson commented 1 year ago

The run-at context menu option works awesome. Thanks.

gormster commented 6 months ago

Is this being worked on actively? I've got a draft implementation here, if you're interested: https://github.com/gormster/userscripts/tree/issue/230

ACTCD commented 6 months ago

@gormster Yes, the issue is on the task list, I think there's a refactoring there regarding scripts uuid and injection process and popup loading process as a prerequisite. So I haven't started this work yet.

Thanks for trying to implement this feat, I did a cursory review and as I mentioned in the comment, this was the first issue I noticed. My previous thought was to store that temporary data in the content script, as well as other related data, such as @resource, I think an updated integrated design would be a lot neater.

We could discuss more if you're interested.

gormster commented 6 months ago

Ah, didn't realise that, but I did think about it when I noticed there were no other JS variables being used in the background script…

Storing the data entirely in the content script would definitely be neater… I'm trying to remember why I didn't implement it that way. I definitely tried! I think it was mainly because originally, the popup page only talks to the background script, which only talks to the native extension process. I don't think it ever talks directly to the content script?

Except, of course, now it does, to actually execute the registered commands… so yeah, can definitely simplify this by just asking the tab for the list of commands instead of the background page. That would be a lot cleaner!

(Aside: do we care about refreshing the popup page when a new command is registered? It can, hypothetically, happen at any time, though I doubt many user scripts are adding commands at any time besides their initial run.)

gormster commented 6 months ago

done! that was actually fairly straightforward, and got rid of a ton of complexity. hooray!

ACTCD commented 6 months ago

@gormster Good quick work! I will leave a new comment when I review again.

But I can answer couple questions first (that you probably already know)

originally, the popup page only talks to the background script, which only talks to the native extension process. I don't think it ever talks directly to the content script?

Well, popup page have no different from background pages. They all are privileged pages that can communicate with native layer or content scripts.

do we care about refreshing the popup page when a new command is registered?

I think popup should only load the currently registered items, not update them anytime after opening, that brings unnecessary re-rendering.

ACTCD commented 6 months ago

@gormster I reviewed your implementation again and it looks mostly fine.

But at the same time I also see some of minor issues, and I'm sorry I don't have time to point them all out at the moment. I just briefly tried to build your branch, looks like it's working as expected. but it looks like the UI could use some improvement, but that may need to be combined with another popup refactoring effort (#447).

I'm working full steam ahead on another refactoring effort regarding the settings UI. So I can't think too much about it at the moment. I might prefer to consider these issues carefully when doing related refactoring designs later.

Again, I'm sorry I didn't provide much useful new information. And thank you for your efforts on this issue.

HimbeersaftLP commented 6 months ago

Aside: do we care about refreshing the popup page when a new command is registered? It can, hypothetically, happen at any time, though I doubt many user scripts are adding commands at any time besides their initial run.

Some scripts might be adding commands asynchronously or, if they are used on a SPA, might add them on demand depending on the current page. (Violentmonkey even has GM.unregisterMenuCommand)

ACTCD commented 6 months ago

@HimbeersaftLP I think any changes (register or unregister) should take effect the next time the user opens the popup, rather than dynamically re-rendering the current popup view, which would cause unnecessary flickering or accidental clicks by the user.

HimbeersaftLP commented 6 months ago

@ACTCD Ohh, I misunderstood that part, yeah, that is probably fine, unless some weird Userscript tries to make a menu navigation with menucommands lol

gormster commented 1 month ago

@ACTCD It's been a while since I worked on this - I just updated my branch to include the latest from upstream, and also fix a couple issues.

That said (I hope life's calmed down a bit since January?) can you have another look at it, and maybe outline the issues you talked about?

ACTCD commented 1 month ago

@gormster I've actually started refactoring popup since April and just switched to svelte5.

Actually like I said before, I hope to include that feature UI design in this refactoring.

I'll take a look at the update you mentioned later.

But if you have a Matrix ID, maybe you'd like to join our development room and we can discuss it.

Thank you for your interest and the effort you put into this.