Features/interface fixes

howardjones / network-weathermap

Network Weathermap draws diagrams from data

http://www.network-weathermap.com/

MIT License

426 stars 94 forks source link

Features/interface fixes #157

Closed RutgerLubbers closed 5 years ago

RutgerLubbers commented 5 years ago

Hi Howard,

as per previous pull request, this PR with some functionality fixes. We've bundled them together.

The changes are: Management:

add group
remove group
add map to group
remove map from group
disable map

Weathermap:

fix overlib: show graph next to cursor when hovering
fix clicking on map: go to graphs
fix showing of map?
show groups as tab (design / css)

General:

css consistency (reload, order, etc)

These changes may not be as you would like to see the final release, but it is working now (a bit more :-) ).

Kind regards,

Rutger Lubbers

howardjones commented 5 years ago

Wow, this is a big one! Thanks! It'll take a little while to go through, but a couple of immediate comments: 1) What's Q24-Readme? 2) You're right about the approach not being the intended one, but hey, I haven't really documented that either. This will work. (specifically - the PDO queries you've added are already in MapManager which is intended to be the home for all the non-Cacti-specific parts of the management plugin)

This looks great though, I'll be reading some more later tonight!

howardjones commented 5 years ago

Rutger - one other thing that I thought I'd already done, but it turns out I haven't yet. I'm planning to change the license for Weathermap from GPL to MIT. Since this PR is the first major code contribution, I thought I should let you know before anything gets merged, in case that is a problem for you.

RutgerLubbers commented 5 years ago

Howard, we're fine with the licence change.

Rutger

pautiina commented 5 years ago

I think it can be merge and then change by you requests

pautiina commented 5 years ago

@RutgerLubbers can you change?

howardjones commented 5 years ago

I merged and then fixed the permissions.

pautiina commented 5 years ago

Thith it right

howardjones commented 5 years ago

Although it doesn't actually pass the tests. You ran the tests, right, Rutger?

netniV commented 5 years ago

@howardjones that's still the ext-snmp error we haven't managed to resolve.

howardjones commented 5 years ago

Fixed - the default group ID for a new map should be 1 not 0.

howardjones commented 5 years ago

No, this was a unit test failing that previously didn’t. Travis is building cleanly now.

https://travis-ci.org/howardjones/network-weathermap/jobs/437817023

On Fri, 5 Oct 2018 at 22:24, Mark Brugnoli-Vinten notifications@github.com wrote:

@howardjones https://github.com/howardjones that's still the ext-snmp error we haven't managed to resolve.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/howardjones/network-weathermap/pull/157#issuecomment-427502902, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3sJRhDhEwbcDrhRUsE171zN5EB2ft_ks5uh830gaJpZM4Wry5n .

netniV commented 5 years ago

I didn't see that one 👍

howardjones commented 5 years ago

Anyway - UI is moving forwards... thanks, Rutger & co! :-)