this topic probably deserves a vote, but before opening one I’d like to discuss the idea first. So I noticed that we have 2 versions of the `#verticalNavigation` velocity macro: a “standard” one provided in macros.vm and one specific to AdminSheet for the Administration menu.
The signature of those macro are very close, but apparently the one from AdminSheet received more love than the standard one and is clearly more advanced and better maintained. I’m not sure why they diverged in the past and why we didn’t reused the standard one but my proposal here is to factorize them to only keep one.
Since the one from AdminSheet is more advanced I’d keep that one and make it standard, enabling some of its feature through the `$options` parameter: e.g. the search field would be enabled through a switch in the option.
The main problem I’m foreseeing is about the styling: the macros don’t share same DOM structure neither same CSS classes, so a custom style put on the standard macro would be probably broken.
About usages of the standard macro, AFAICS the macro is not used in xwiki-contrib, and in XWiki Standard it’s used for the user profile tabs, and for the XWiki syntax pages. I haven’t found any other usage. So my main concern is about custom style of user profile tabs, so that’s probably what might be the most impacted.
I actually just found out that there’s a 3rd implementation, not of `verticalNavigation` but of `verticalNavigationItem` which is at the core of the macro in fact, this time in flamingo skin macros.vm file… And this implementation is even different from the 2 others: it adds the capability to display an icon compared to the standard one.
So for me it emphasizes even more the need to factorize and simplify all this.
It’s not only CSS. Someone might have used JavaScript to inject custom entries in the user profile menu. That code would break when changing the HTML structure. Whether the HTML structure is an API or not is a bit blurry, but the risk of breakage exists. Which is probably why I opted for introducing a new macro when I worked on improving the Wiki Administration.
The initial goal was to have:
web/macros.vm: a set of generic / reusable Velocity macros that don’t depend on the skin
flamingo/macros.vm: can overwrite generic macros with an implementation that uses elements provided by the flamingo skin (i.e. Bootstrap and Glyphicons)
If you compare the implementation of verticalNavigationItem from the moment we improved the administration menu:
because Glyphicon set was provided by Bootstrap which was a dependency of the Flamingo skin. Since then, we moved to use Bootstrap everywhere, making it the de facto XWiki’s design system.
Conceptually, I still think it makes sense to separate macros that don’t depend on the skin from macros that depend (especially if we want to continue supporting the ability to configure a different skin). But we need to review the flamingo/macros.vm to see if they are still skin-dependent. In this particular case, if the Glyphicon usage is replaced with the icon theme script service then the implementation doesn’t depend on the skin anymore, so we can drop the override.
To get back to the current topic, I agree we should unify the #verticalNavigation implementations, in order to reduce code duplication and to have a more consistent UI.