In the T214715: [Spike] How should menus work in Minerva? we did a research on Menu building in Minerva skin. We came to conclusion, that we can and we should construct all menus in the same way. This approach will
- give us more flexibility, it should be easy to modify/replace menu elements
- extract PageActions logic from SkinMinerva class
- allow us to unit test menu building logic
- reduce the way how we build menus
In the T216152: AMC Navigation - add new links to main menu with click tracking we re-organized the code and used the Builder pattern to build MainMenu. In the same time we merged T216418: AMC Navigation - overflow menu which introduces the new overflow menu.
We should refactor the PageActions menu building to match new way how menus are built.
The proof-of-concept is available on gerrit: rSMIN9909b2d05d2d: POC: PageActions menu built using Builder pattern
Acceptance Criteria
- PageActions menu building logic is removed from SkinMinerva class
- PageActions menu is built using Builder pattern. Menu/Group and Menu/IMenuElement are used as building blocks
- Any extensions that inject menu elements to the Page actions bar still works properly
- Shared logic from SkinMinerva class (if any) should live as a separate service that is used both in SkinMinerva and PageActions menu builder
- The Page Actions menu works same way as it used to
QA Steps
On Beta cluster verify:
- PageActions works the same as it used to for anonymous user
- PageActions works the same for AMC user
- PageActions works the same for non-amc user
- PageActions overflow menus work the same for NS_MAIN and NS_USER namespaces
Additionally on local env/reconfigured staging please verify
- Toolbar menu works as expected when the AMC mode is disabled (Important) - staging env has AMC mode disabled for the test
- PageActions works properly if OverflowMenu is not enabled
QA Results
| AC | Status | Details |
|---|---|---|
| 1 | ✅ | T221792#5289083 |
| 2 | ✅ | T221792#5289083 |
| 3 | ✅ | T221792#5289083 |
| 4 | ✅ | T221792#5289083 |
| 5 | ✅ | T221792#5289083 |
| 6 | ✅ | T221792#5289083 |
