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

Redesign alert template modal #9364

Merged
merged 6 commits into from Oct 25, 2018

Conversation

Projects
None yet
4 participants
@crazy-max
Member

crazy-max commented Oct 22, 2018

  • "Attach template rules" modal merged to "Alert template" modal
  • Use select2
  • Autocompletion available
  • Fix orphans occurrences in db while removing rule or template
  • Use 'pre' font family for alert template textarea

Preview

image

image

image

@TheGreatDoc

This comment has been minimized.

Contributor

TheGreatDoc commented Oct 22, 2018

Nice! But there is a little issue (that was fixed by me :P) and now is back.

You can select the same rule in two different templates. That will make the rule to use the template with lowest ID.

What I did is a check and disable rules to be selected and show in what template is used in case you want to switch the template for a rule from one to another.

@crazy-max

This comment has been minimized.

Member

crazy-max commented Oct 22, 2018

@TheGreatDoc Yes indeed, is it better now ?

image

Show resolved Hide resolved html/includes/modal/alert_template.inc.php Outdated
Show resolved Hide resolved html/js/bootstrap-select.min.js Outdated

@laf laf added this to the 1.45 milestone Oct 22, 2018

crazy-max added some commits Oct 22, 2018

@crazy-max

This comment has been minimized.

Member

crazy-max commented Oct 23, 2018

@laf @murrant Ok I have mimic bootstrap-select using select2 :

image

What do you think ?

@laf

laf approved these changes Oct 23, 2018

LGTM + Tested ok.

Nice work :)

Btw, working from your master branch isn't a good idea, always best to branch off first.

@laf laf requested a review from murrant Oct 23, 2018

@crazy-max

This comment has been minimized.

Member

crazy-max commented Oct 23, 2018

@laf For this kind of feature yeah you right

@murrant

Great :)

@murrant murrant merged commit 00b752a into librenms:master Oct 25, 2018

2 of 3 checks passed

codeclimate 2 issues to fix
Details
WIP ready for review
Details
license/cla Contributor License Agreement is signed.
Details
@crazy-max

This comment has been minimized.

Member

crazy-max commented Oct 25, 2018

🎉

@murrant

This comment has been minimized.

Member

murrant commented Oct 28, 2018

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/v1-45-release-changelog-october-2018/6016/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment