search

wikimedia-gadgets / MoreMenu

The missing navigation system for MediaWiki
https://meta.wikimedia.org/wiki/Special:MyLanguage/MoreMenu
MIT License
7 stars 13 forks source link

MoreMenu.js: hides core Menu if empty #9

Closed pols12 closed 1 year ago

pols12 commented 3 years ago

Hides div#p-cactions which used to contain “Move” link and which is now empty.

Fixes #6

MusikAnimal commented 3 years ago

Unfortunately it's not this simple. There's no guarantee that some other script will initialize after MoreMenu and try to add a link, for example.

Instead, we use a MutationObserverer to automatically show/hide the More menu based on whether it's empty. This should be working now.

In addition, MoreMenu tries to intelligently learn if any other scripts change the More menu, and based on that it will decide whether it should leave the More menu unhidden. For that logic, see https://github.com/wikimedia-gadgets/MoreMenu/blob/master/src/MoreMenu.js#L878-L902

That said, are you still seeing the empty More menu, @pols12 ?

pols12 commented 3 years ago

Unfortunately it's not this simple.

😢

That said, are you still seeing the empty More menu, @pols12 ?

Yes, I do. However, I used to enable PurgeTab gadget which added a “Purge” link in More menu. So observeCactionsMenu() may have populated mmNativeMenuUsage local storage key. I just removed it (it was valued at 5) and now More menu is correctly hidden (thanks for pointing this!).

Do you think it would interesting/feasible to make this counter automatically decrease with time?

MusikAnimal commented 3 years ago

Do you think it would interesting/feasible to make this counter automatically decrease with time?

Yes, I think that makes sense. Maybe it should have an expiry, say a week or so. I'll try to look into this soon, unless you care to rework this PR :) Thanks for your feedback either way!

pols12 commented 3 years ago

Hello MusikAnimal, I have reworked the code. Do you have any time for reviewing it? (sorry for bad commit history)

MusikAnimal commented 1 year ago

Dang, I'm so sorry it took so long to get back to this! I'm now cleaning up all the PRs and realized this one has been around for years. it's just hard to test.

Also, things have changed... now we have a setExpires() via mw.storage, so there's no need to use an object. We'll just set an expiry and when it expires, it's as if there was no localStorage item to begin with, effectively doing what we want.

I'll rework the code to use this new approach, but in the spirit of collaboration, I'm going to merge your PR as-is so you at least get a spot in the git history :)

Thanks for contributing, and apologies again for never getting back to you!