-
Notifications
You must be signed in to change notification settings - Fork 356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introducing pluggable menus #1454
Conversation
Checked commits skateman/manageiq-ui-classic@52e71a5~...c020733 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 app/presenters/menu/yaml_loader.rb
|
Cool. Pretty minimal changes to reach the goal. 👍 |
I think that there's no need to spend time on the "future steps" at this point. Rather we can attack the other items on the minimal requirements list for a pluggable UI. |
Sure, I wasn't thinking about doing anything more in this PR. However, I'll create a separate one where we have our extended Rails Engine with some plugin definition DSL where we can register menus and other stuff. Marking it as a non-wip... |
Have you tested the old custom menu loading (Insights)? |
@martinpovolny I copied some productization ymls to my development setup, it's showing up in the |
@martinpovolny tested, it works |
These changes will allow us to declare menu items in external plugins. I implemented a new
Menu::CustomLoader
singleton, in which you can.register
your menu items. For now this can be done from aninitializer
block declared in a Rails Engine.The
.register
method can take aMenu::Section
or aMenu::Item
as an argument with the same syntax as we're using in theMenu::DefaultMenu
.For example this will create a new Test section with a Test item pointing to the
/test
URL:In the future I'd like to take out the menu declarations into a better place, e.g.
app/menus
and have some kind of auto-registering feature inside the engine. The syntax of the menu declaration should be also DSL-ized and used in theMenu::DefaultMenu
too.@martinpovolny what do you think?