Choose the menu insertion points?

[baskerville]

I was wondering if it would be possible to add the ability to choose where the menu items are inserted? Or to have better defaults: ideally the new items should be isolated from the existing items by a black separator. Looking at the code, I'm starting to think that you simply don't have a choice?

[pgaskin]

Not really. One reason I chose the items I did is since they are unlikely to change, and since they won't ever be completely eliminated by one of the menu patches from kobopatch.

The only proper way to approach this which I can think of would be to just intercept the pointer to the menu itself (i.e. get the parent widget), and then add the other menu items separately. I might do this in the future to implement live config-updating, but I'm unsure if it's worth the extra complexity (and thus, breakage) in the menu code (which I'm trying to keep as safe and version-independent as possible). See geek1011/kobo-plugin-experiments#3 for some discussion about this (I've invited you to the repo).

[pgaskin]

I can probably fix the separator color, that's probably just a matter of patching the vtables correctly or adding the separator manually. There's also some discussion on this in geek1011/kobo-plugin-experiments#3.

[baskerville]

Regarding the main menu, isn't Settings as stable as Help?

In which case, wouldn't it be better to insert the new items before Settings?

There would then be a black separator before the first item and if you find a way to add one after the last, you would end up with a subsection that contains the new items.

I finally understood the code: you're intercepting the function that creates menu items! And therefore ismm and isrm are both false most of the time.

[pgaskin]

Regarding the main menu, isn't Settings as stable as Help? In which case, wouldn't it be better to insert the new items before Settings? There would then be a black separator before the first item and if you find a way to add one after the last, you would end up with a subsection that contains the new items.

Yes, and I'm not sure yet. Is there any reason for this other than the color? If I figure out how to change the color for one, it will be easy enough to do it for all of them, including the original items.

Update: It seems that to change the separator color, I just need to add a LightMenuSeparator or BoldMenuSeparator action.

I finally understood the code: you're intercepting the function that creates menu items! And therefore ismm and isrm are both false most of the time.

Yes, that's it. If you want to be even more boggled, try and understand the dlhook code. That's has even more subtleties to it.

[baskerville]

There would then be a black separator before the first item and if you find a way to add one after the last, you would end up with a subsection that contains the new items.

Is there any reason for this other than the color?

I think they can be several levels of improvement:

1. Add the new items at the beginning or the end of a section: it would make it easier to identify the new items. When I first tried NickelMenu, it took me some time to figure out where the new items were.
2. Add a black separator so that the new items appear in their own section: the eye of the user can then quickly jump to the black separator that precedes the first item.
3. Make the inter-item separator match the one used by the existing items: it would improve the consistency and prevent the user from being distracted by this minor ui incoherence.

[pgaskin]

@baskerville, can you try this: https://github.com/geek1011/NickelMenu/suites/666420015/artifacts/5937533?

[NiLuJe]

Yup, that works on my end, :+1:

(EDIT: As opposed to https://github.com/geek1011/kobo-plugin-experiments/issues/3#issuecomment-619479552)

[baskerville]

That's impressive!

Now that you have full control of the separators, I would suggest adding the new items after the last existing item (Help for the main menu and Dictionary for the reader menu). It will match the intuition of the user about the items being added at the end. And the eye of the user will probably initially jump where the end of the untouched menu was.

This would also simplify the code:

• To add the items after the entry point you just need to move the last function call before the loop.
• You need to return early if !is_mm && !is_rm.
• You can then unconditionally add a black separator before the loop.
• Within the loop, the separator's color will only depend on is_mm.

[pgaskin]

I probably won't be making it the last item for stability reasons. Currently, it wouldn't be an issue, but in the future, if there ever is any dependency on the function call order, it could potentially cause crashes. Remember, we are intercepting the function call, so it's easier (and more stable) to add stuff before the real function call.