Add settings checkbox for removing context menu item

[h-h-h-h]

Installing various add-ons can easily clutter up the context menu, even if some items are just present when text is selected.

Please add a checkbox as: [x] Show context menu item for selected text. You can keep the current default.

I think, when the following already present checkbox is checked: [x] Automatically use the text selected on the website, the new checkbox should be disabled and unchecked.

[rugk]

You know that one only sees the context menu item if text is selected and/or if you click on a link?

So it's at least not shown for all cases.
And thus, IMHO, quite discrete…

---

That said, I would be fine with such a checkbox (enabled by default, as you said). (PRs appreciated…)

I am not fine with automatically disabling/modifying it when another option is changed though. The correlation between the settings may not obvious to the user and there are valid use-cases for having both settings enabled.

[shanirub]

Hi there, I would like to work on this issue.

The task as i understand it includes the following steps:

1. Edit options.html, so that the new checkbox will be displayed.
2. Edit DefaultSettings.js, so that this option will be enabled as default (as rugk requested).
3. Edit messages.json files for each language (or maybe this should come afterwards, as a different task?).
4. Edit InitQrCode.js: write the logic behind this option.

As a beginner i would love to get some feedback:
Did i forget anything? Any advice?
Thanks.

[rugk]

Hi @shanirub ,
great, yes, feel free to take this issue. 😃

1. Yep. For help see the module doc for settings: https://github.com/TinyWebEx/AutomaticSettings
2. Yep, exactly.
3. When you do this hardly matters, but yes, the new options should be localized. Again module doc: https://github.com/TinyWebEx/Localizer (Of course, you can only translate it for the languages you speak by yourself.)
4. Well..., no. The real pointer for you would be https://github.com/rugk/offline-qr-code/blob/master/src/background/modules/ContextMenu.js

[shanirub]

• Displaying option as a check box.
  options.html
• Implementing the logic.
  ContextMenu.js
• Option should be enabled as default.
  DefaultSettings.js
• Updating localization.

[c3ho]

Hi, I noticed this hasn't been updated since July 22. May I work on the issue?
I'm a first time contributor, so please bear with me.

[rugk]

@shanirub already did some work on this, as it seems. So let's wait for a reply here.
Having a quick look at the fork, the changes are apparently in the master branch here: https://github.com/shanirub/offline-qr-code/commits/master (not reviewed!)

So assigning @shanirub for getting a reply whether @c3ho can/should take over this change or not?

[shanirub]

@rugk I would very much like to continue working on this issue.
@c3ho sorry.

[jn64]

Hope to see this feature, because Firefox default access key for Inspect Element is also Q.

Alternatively allow changing the access key for this addon to R

[rugk]

Alternatively allow changing the access key for this addon to R

For simplicity reasons, we'd only want to have one fixed and value and it should be a word that is included in the context menu text. In any case, however, your underlying issue seems to be valid/a good point, so please open a new issue for that. (As it may be fixed independently to this one, as you've noticed by yourself.)

[rugk]

@shanirub If you need any pointers/help or have questions, feel free to ask.

[shanirub]

@shanirub If you need any pointers/help or have questions, feel free to ask.

Thank you, that would be great. I could use some help+pointers.

[rugk]

Well you've already collected some things to do and you already did something. I've also pointed you to the modules that are documented on how they work.
Feel free to ask any questions, if some come up.

As far as I see you already have the setting, you just need to read the setting in the module (done with https://github.com/TinyWebEx/AddonSettings) and apply it.

[rugk]

@shanirub Are you still interested in it? If not, that's no problem, just add a notice, so I know what the current status of this issue is.

[schmelto]

Hey there, is this one an easy issue for my a looking for the first contribution?
If yes I'd like to address this issue.
I only need to know JavaScript?

[rugk]

Hi @schmelto sorry for the delay, I was busy.

Yeah, feel free to take this on. It should be easy as it is just about adding a settings options and the logic behind this option.
The official contributing guide should help you, as always: https://github.com/TinyWebEx/common/blob/master/CONTRIBUTING.md

I only need to know JavaScript?

Yes, mostly. Of course you HTML/CSS knowledge may be useful too. (though you likely don't need CSS knowledge for this task)
Also the docs, I've pointed to before are useful as you need to add a settings option.

Most knowledge you'll (need to/will) learn is about the WebExtensions API. See https://developer.mozilla.org/docs/Mozilla/Add-ons/WebExtensions/user_interface/Context_menu_items for a starting point.

If you have any questions, feel free to ask.

[rugk]

Also sorry to the others who I had to unassign/delay from this issue, I'll leave this to @schmelto for now as @schmelto was the last one to comment here and show interest in implementing this feature.
Of course, nothing prevents you from working together or so. Also check out the old repos from the contributors or so.

[schmelto]

Hey @rugk,
I need some assistance.

I managed to add a checkbox into the options.html

And add this at the top after the Colored Icon:

<input class="setting save-on-change" type="checkbox" id="popupShowContextMenu" name="popupShowContextMenu">

• Is this the right position for this?
• Do I need to make the default checking in the options.js?

Kind regards
schmelto

[rugk]

Is this the right position for this?

Well… this can always be changed later, but I'd place it below the heading "Add-on behaviour".

Do I need to make the default checking in the options.js?

Yes, technically you always need to specify a default.
And if you want to have it enabled by default (which I'd say we want, depending on how you describe the option), yes you need to set it to true.

[schmelto]

Maybe you can check on my first try to resolve this issue :) thx

[Hochiss]

Any follow-up gents?

[rugk]

@schmelto has closed their pull request, so if anyone wants to try it and is still interested, this is ready to be taken up by somebody.

[starborne-nova]

Hi there. I'd like to take the issue if it's currently unassigned and still a valid issue

[starborne-nova]

log2022-2-21_21-46-22.txt

I've managed to integrate an option for disabling the context menu, however I haven't been able to make the context menu appear at all. Was hoping I could get some help figuring out what I am doing wrong. I've attached the console log from trying to get it to work.

And here's the changes I've made so far

starborne-nova@c95a4de

[rugk]

@starborne-nova From having a quick look the code changes all looks fine? Did you make sure to clone the repo with submodules or check them out afterwards?

In any case, I'd appreciate a PR, so the option can be integrated.