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

Webui: Allow Submenus in Plugin Menu by removing the scrollable-menu … #8762

Merged
merged 1 commit into from May 24, 2018

Conversation

Projects
None yet
6 participants
@PipoCanaja
Copy link
Contributor

PipoCanaja commented May 24, 2018

Hello

The "scrollable-menu" class appears incompatible with nested submenus. Removing it for Plugin submenu would allow a plugin to use SubMenus itself.

Use case: WeatherMap would list in a submenu all the weathermaps defined, allowing a quick access.

The patch for weathermap would come if this one is accepted.

Bye
PipoCanaja

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

@kkrumm1 kkrumm1 added the WebUI label May 24, 2018

@laf

This comment has been minimized.

Copy link
Member

laf commented May 24, 2018

To be fair you should just add the weathermap update to this PR so we can see it in action.

@PipoCanaja

This comment has been minimized.

Copy link
Contributor Author

PipoCanaja commented May 24, 2018

I will definitly push it. I am trying to understand how it works in order to integrate it cleanly in the plugin-menu (and safely).
Here is a capture of how it goes so far.

weathermap

By the way, Weathermap is in another git tree (librenms-plugins) and not this one. How would you like to proceed ? push it to my github and then you clone it ?

PipoCanaja

@laf

This comment has been minimized.

Copy link
Member

laf commented May 24, 2018

By the way, Weathermap is in another git tree (librenms-plugins) and not this one. How would you like to proceed ? push it to my github and then you clone it ?

Just do a pull request as you would have done if this was merged.

In fact, I'll just merge this.

@laf

laf approved these changes May 24, 2018

Copy link
Member

laf left a comment

LGTM

@laf laf merged commit f148239 into librenms:master May 24, 2018

3 checks passed

WIP ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@PipoCanaja PipoCanaja deleted the PipoCanaja:Webui-SubmenuInPluginMenu branch May 24, 2018

@PipoCanaja

This comment has been minimized.

Copy link
Contributor Author

PipoCanaja commented May 24, 2018

OK. So I'll do a PR with Weathermap right into the standard https://github.com/librenms/librenms/ as soon as I have something "not too bad".

@laf

This comment has been minimized.

Copy link
Member

laf commented May 24, 2018

If it's the weathermap plugin you're editing then: https://github.com/librenms-plugins/Weathermap/

@Majesticops

This comment has been minimized.

Copy link

Majesticops commented May 31, 2018

I have updated my install but I cannot see the weathermap maps submenu menu under Plugins-->Weathermap as it shown above

Do I need to update my weathermap install.

@laf

This comment has been minimized.

Copy link
Member

laf commented May 31, 2018

I don't think a PR has been done for the actual weathermap yet.

@DonovanEsterhuizen

This comment has been minimized.

Copy link

DonovanEsterhuizen commented Jun 1, 2018

Thanks will wait for the PR for wearthermap.

@zorun

This comment has been minimized.

Copy link

zorun commented Jun 2, 2018

@PipoCanaja can't wait for your second PR to come, it would be a nice functionality :)
(I miss it from the observium integration)

mattie47 added a commit to mattie47/librenms that referenced this pull request Jul 2, 2018

webui: Allow Submenus in Plugin Menu by removing the scrollable-menu … (
librenms#8762)

Hello

The "scrollable-menu" class appears incompatible with nested submenus. Removing it for Plugin submenu would allow a plugin to use SubMenus itself. 

Use case: WeatherMap would list in a submenu all the weathermaps defined, allowing a quick access.

The patch for weathermap would come if this one is accepted.

Bye
PipoCanaja

DO NOT DELETE THIS TEXT

#### Please note

> Please read this information carefully. You can run `./scripts/pre-commit.php` to check your code before submitting.

- [X] Have you followed our [code guidelines?](http://docs.librenms.org/Developing/Code-Guidelines/)

#### Testers

If you would like to test this pull request then please run: `./scripts/github-apply <pr_id>`, i.e `./scripts/github-apply 5926`
@PipoCanaja

This comment has been minimized.

Copy link
Contributor Author

PipoCanaja commented Jul 11, 2018

PR is on the way in the librenms-plugins repo. You can follow it there :
librenms-plugins/Weathermap#51

Bye
PipoCanaja

@lock lock bot locked as resolved and limited conversation to collaborators Sep 9, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.