Skip to content
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

Menu::Manager - support adding to existing sections #3417

Closed
wants to merge 1 commit into from
Closed

Menu::Manager - support adding to existing sections #3417

wants to merge 1 commit into from

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Feb 16, 2018

Right now, plugins can add new sections and items in the menu, but can't append to existing sections.

Menu sections already have ids, with this PR, if you create an already existing section in a plugin, it will get merged with the existing one.

Cc @ZitaNemeckova, @priley86
@martinpovolny please review

This is so that v2v can replace priley86@5367072 with an in-plugin solution.

sections have ids, so if you define an already existing section in the plugin, the items will get added to it
@himdel
Copy link
Contributor Author

himdel commented Feb 16, 2018

Testing:

diff --git a/lib/manageiq/ui/classic/engine.rb b/lib/manageiq/ui/classic/engine.rb
index 60431e639..aadf4b4c4 100644
--- a/lib/manageiq/ui/classic/engine.rb
+++ b/lib/manageiq/ui/classic/engine.rb
@@ -57,6 +57,17 @@ module ManageIQ
         def vmdb_plugin?
           true
         end
+
+        initializer 'plugin' do
+          Menu::CustomLoader.register(
+            Menu::Section.new(:compute, N_("Compute"), 'pficon pficon-cpu', [
+              Menu::Section.new(:migration, N_("Migration"), 'fa fa-plus', [
+                Menu::Item.new('overview', N_('Overview'), 'miq_report', {:feature => 'miq_report', :any => true}, '/migration')
+              ])
+            ])
+          )
+        end
+
       end
     end
   end

Make sure Migration appears under (=inside) Compute.

@miq-bot
Copy link
Member

miq-bot commented Feb 16, 2018

Checked commit https://github.com/himdel/manageiq-ui-classic/commit/c0e5b78055eddc106cc0267764f2eccc18439a43 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

@ZitaNemeckova
Copy link
Contributor

Tested with https://github.com/priley86/miq_v2v_ui_plugin/pull/48. Works in UI. LGTM 👍

@martinpovolny
Copy link
Member

martinpovolny commented Feb 16, 2018

well I have this ready here: #3347 and I actually said so on the standups a couple of times ;-)

I just kept it marked as "WIP" because I have no test coverage. But there's no tests here either ;-)

@himdel
Copy link
Contributor Author

himdel commented Feb 16, 2018

Well, standups are audio-only :)

So.. yours is longer so probably does more :). Should I close this one then? (as in, can we merge the other one soon? :))

And how would it work, something llike..

Menu::CustomLoader.register(
  Menu::Section.new(:migration, N_("Migration"), 'fa fa-plus', [
    Menu::Item.new('overview', N_('Overview'), 'miq_report', {:feature => 'miq_report', :any => true}, '/migration')
  ], nil, nil, nil, nil, :compute)
)

?

EDIT: answered in the commit description (07368c5), short answer: yes.

@himdel
Copy link
Contributor Author

himdel commented Feb 16, 2018

Closed in favor of #3347

@himdel himdel closed this Feb 16, 2018
@himdel himdel deleted the register-duplicate-section branch February 16, 2018 13:54
@priley86
Copy link
Member

thanks very much @ZitaNemeckova @martinpovolny @himdel ... nice work! 👍

happy to adopt this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants