Skip to content

[JENKINS-59684] Support "alert-info" for admin monitor in popup - #4276

Merged
oleg-nenashev merged 2 commits into
jenkinsci:masterfrom
Wadeck:JENKINS-59684_monitor_info
Oct 12, 2019
Merged

[JENKINS-59684] Support "alert-info" for admin monitor in popup#4276
oleg-nenashev merged 2 commits into
jenkinsci:masterfrom
Wadeck:JENKINS-59684_monitor_info

Conversation

@Wadeck

@Wadeck Wadeck commented Oct 7, 2019

Copy link
Copy Markdown
Contributor
  • Add support for alert-info and alert-danger also in the popup for admin monitor.
  • Also support the p tags in the message, discovered with the CSRF monitor message being only "half-styled".
Screenshots

Full page mode (ok before and after)

full_page_ok

Popup mode

Before

popup_ko

After

Screenshot_2019-10-07_114211_001

For CSRF monitor, the paragraphs are now also correctly styled

Before

Screenshot_2019-10-07_114815_001

After

Screenshot_2019-10-07_114758_001

For security warnings

Before

Screenshot_2019-10-07_121622_001

After

Screenshot_2019-10-07_121609_001

See JENKINS-59684.

Proposed changelog entries

  • Administrative monitors providing information message are now correctly styled in the popup mode

Submitter checklist

  • JIRA issue is well described
  • Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    * Use the Internal: prefix if the change has no user-visible impact (API, test frameworks, etc.)
  • [n/a] Appropriate autotests or explanation to why this change has no tests
  • [n/a] For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@recena @daniel-beck

@Wadeck Wadeck mentioned this pull request Oct 7, 2019
13 tasks
@res0nance res0nance added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Oct 7, 2019

@jvz jvz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this!

@daniel-beck

Copy link
Copy Markdown
Member

Why does this edit style.css when #2857 worked on bootstrap.css etc.?

@oleg-nenashev oleg-nenashev added the web-ui The PR includes WebUI changes which may need special expertise label Oct 8, 2019
@Wadeck

Wadeck commented Oct 8, 2019

Copy link
Copy Markdown
Contributor Author

The alert-warning that was applied before comes from #1847.

The bootstrap.css is applied only to manage.jelly, don't ask me why. https://github.com/Wadeck/jenkins/blob/4d65572c6db9099ebcf8d2222159c069996670f9/core/src/main/resources/jenkins/model/Jenkins/manage.jelly#L33 and #2857

@oleg-nenashev oleg-nenashev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 , thanks!
I was thinking about delivering upgrade guide heads-up through update centers, and this is a great foundation improvement for it

I plan to merge it tomorrow if no negative feedback

@oleg-nenashev oleg-nenashev added developer Changes which impact plugin developers and removed developer Changes which impact plugin developers labels Oct 11, 2019
@daniel-beck daniel-beck added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Oct 11, 2019
@oleg-nenashev
oleg-nenashev merged commit ab46f02 into jenkinsci:master Oct 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted web-ui The PR includes WebUI changes which may need special expertise

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants