Omitting default actions if already provided about electron-context-menu HOT 7 OPEN

Makes sense, but should be based on id, not the label. I don't think we need an option for it. Should the replaced item take the original position or be added to the start/end depending on whether prepend/append was used? I'm leaning towards the latter.

We currently have a shouldShowMenu option, so it could also maybe be useful with one for menu items called shouldShowMenuItem? In case you want to dynamically decide which of the items to show.

from electron-context-menu.

but one would want to localize this library too

It would be weird to have to translate the menu item here when you don't use it and instead override it with your. These are many reasons why as stable ID is better.

from electron-context-menu.

Makes sense, but should be based on id, not the label.

Maybe if no id is explicitly provided but the label is the same as a default menu item that default menu item should still be omitted? I just can't imagine a scenario where one would want 2 "Copy" menu items like shown in the screenshot.

Should the replaced item take the original position or be added to the start/end depending on whether prepend/append was used?

I'd say the latter too.

We currently have a shouldShowMenu option, so it could also maybe be useful with one for menu items called shouldShowMenuItem? In case you want to dynamically decide which of the items to show.

Maybe, I'm currently updating the menus inside shouldShowMenu before showing them (ref).

If there was a shouldShowMenuItem item option I think that could be useful in some scenarios, but I would personally not use it as whether or not some menu items should be shown may depend on some computation that I'd rather perform once for all items rather than for each time and I'd rather perform it next to the logic for updating the menu itself rather than in a different place.

Maybe an updateMenu option, maybe called with the entire menu template before showing it, could be useful too.

I'd be happy to submit a PR implementing these features 👍

from electron-context-menu.

Maybe if no id is explicitly provided but the label is the same as a default menu item that default menu item should still be omitted? I just can't imagine a scenario where one would want 2 "Copy" menu items like shown in the screenshot.

The reason I want to base it on id is that menu items could be localized and that could create ambiguity. It's just easier to reason about if it's only based on the id.

Maybe an updateMenu option, maybe called with the entire menu template before showing it, could be useful too.

👍 That actually sounds more useful and flexible than my shouldShowMenuItem idea.

from electron-context-menu.

The reason I want to base it on id is that menu items could be localized and that could create ambiguity. It's just easier to reason about if it's only based on the id.

I see your point, but some apps don't deal with i18n at all, and those who do I guess would translate "Copy" from this library and the custom "Copy" item with the same string 🤔

I'll try to get the PR done by the end of the week 👍

from electron-context-menu.

I see your point, but some apps don't deal with i18n at all, and those who do I guess would translate "Copy" from this library and the custom "Copy" item with the same string 🤔

I was referring to depending on overriding the "Copy" item here, then localizing and suddenly the override stops working as the labels no longer match.

from electron-context-menu.

Yeah yeah, but one would want to localize this library too and assuming both instances of "Copy" get localized to the same string (but languages are complicated, maybe this assumption can't be made) the override would still work.

from electron-context-menu.