[JENKINS-73114] avoid conflicts with css classes from bootstrap #9254

mawinter69 · 2024-05-10T13:15:37Z

jenkins and bootstrap both have definitions for alert and alert-warning, alert-info and alert-danger. Bootstrap css definitions are included when e.g. the warning-ng plugin is installed and a job is configured to scan for warnings. The css classes from bootstrap are then chosen instead of the ones for Jenkins for things like the admin monitors. To make things unambiguous, add additional classes prefixed with jenkins- and make use of them in code. Keep the old definitions for backward compatibility with plugins.
Follow-up changes are needed in plugins (Mostly those that define admin monitors) and the design-library

After:
for a plugin message:

for a core message:

See JENKINS-73114.

Originally reported as jenkinsci/customizable-header-plugin#99

Testing done

Manual testing

Create new Jenkins instance, it should have some executors for built-in, this will trigger the corresponding monitor
Visit page /manage/ -> monitors are displayed properly
install warnings-ng plugin
create freestyle job and configure post build action to scan for e.g. stylellint.
run the job once
In the header click on the warning shield -> monitor is displayed properly (If there are monitors from plugins they might not be properly styled here)

Proposed changelog entries

Add new CSS classes to avoid conflicts with CSS classes from bootstrap.

Proposed upgrade guidelines

N/A

Submitter checklist

Give feedback

The Jira issue, if it exists, is well-described.

The Jira issue, if it exists, is well-described.
Options
Successfully updated the issue's project

There was an error updating the issue's project
The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see [examples](https://github.com/jenkins-infra/jenkins.io/blob/master/content/_data/changelogs/weekly.yml)). Fill in the **Proposed upgrade guidelines** section only if there are breaking changes or changes that may require extra steps from users during upgrade.

The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
Options
Successfully updated the issue's project

There was an error updating the issue's project
There is automated testing or an explanation as to why this change has no tests.

There is automated testing or an explanation as to why this change has no tests.
Options
Successfully updated the issue's project

There was an error updating the issue's project
New public classes, fields, and methods are annotated with `@Restricted` or have `@since TODO` Javadocs, as appropriate.

New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
Options
Successfully updated the issue's project

There was an error updating the issue's project
New deprecations are annotated with `@Deprecated(since = "TODO")` or `@Deprecated(forRemoval = true, since = "TODO")`, if applicable.

New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
Options
Successfully updated the issue's project

There was an error updating the issue's project
New or substantially changed JavaScript is not defined inline and does not call `eval` to ease future introduction of Content Security Policy (CSP) directives (see [documentation](https://www.jenkins.io/doc/developer/security/csp/)).

New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
Options
Successfully updated the issue's project

There was an error updating the issue's project
For dependency updates, there are links to external changelogs and, if possible, full differentials.

For dependency updates, there are links to external changelogs and, if possible, full differentials.
Options
Successfully updated the issue's project

There was an error updating the issue's project
For new APIs and extension points, there is a link to at least one consumer.

For new APIs and extension points, there is a link to at least one consumer.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Options

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

Maintainer checklist

Give feedback

There are at least two (2) approvals for the pull request and no outstanding requests for change.

There are at least two (2) approvals for the pull request and no outstanding requests for change.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.

Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Changelog entries in the pull request title and/or **Proposed changelog entries** are accurate, human-readable, and in the imperative mood.

Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Proper changelog labels are set so that the changelog can be generated automatically.

Proper changelog labels are set so that the changelog can be generated automatically.
Options
Successfully updated the issue's project

There was an error updating the issue's project
If the change needs additional upgrade steps from users, the `upgrade-guide-needed` label is set and there is a **Proposed upgrade guidelines** section in the pull request title (see [example](https://github.com/jenkinsci/jenkins/pull/4387)).

If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
Options
Successfully updated the issue's project

There was an error updating the issue's project
If it would make sense to backport the change to LTS, a Jira issue must exist, be a _Bug_ or _Improvement_, and be labeled as `lts-candidate` to be considered (see [query](https://issues.jenkins.io/issues/?filter=12146)).

If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).
Options
Successfully updated the issue's project

There was an error updating the issue's project
Options

jenkins and bootstrap both have definitions for alert and alert-warning, alert-info, alert-danger. Bootstrap css definitions are included when e.g. the warning-ng plugin is installed and a job is configured to scan for warnings. The css classes from bootstrap are then chosen instead of the ones for Jenkins for things like the admin monitors. To make things umambiguous, add additional classes prefixed with `jenkins-` and make use of them in code. Keep the old definitions for backward compatibility with plugins. Followup changes are needed in plugins (Mostly those that define admin monitors) and the design-library

NotMyFault

Thanks!

/label ready-for-merge

This PR is now ready for merge. We will merge it after ~24 hours if there is no negative feedback.
Please see the merge process documentation for more information about the merge process.
Thanks!

timja · 2024-05-18T10:10:43Z

Can you create a PR for the design library to adapt to this please?

mawinter69 · 2024-05-18T11:24:51Z

Can you create a PR for the design library to adapt to this please?

done

MarkEWaite · 2024-05-19T11:49:44Z

@mawinter69 can you also identify the plugins that need to be updated to use the new CSS classes? I think it is better to complete the migration from the old CSS classes to the new CSS classes in the most popular plugins.

mawinter69 · 2024-05-20T11:33:37Z

Should this maybe downported to LTS 2.452.2? Otherwise it will take quite a while until it arrives in an LTS. Plugins would otherwise need to use weekly 2.459. Specifying both the old and new classes doesn't help when bootstrap loaded so it can't really be made forward compatible

mawinter69 · 2024-05-20T12:12:22Z

I Identified following plugins making use of the any of the alert-* classes. That doesn't necessarily mean they need to be adjusted, e.g. pipeline-stage-view seems to have a bootstrap3 included and relies on the definitions there.

active-directory
alauda-devopy-synvc
artifact-manager-s3
atlassian-bitbucket-server-integration
audit-trail
azure-ad
azure-vm-agents
blue-ocean
branch-api
cloudbees-jenkins-advisor
collabnet
configuration-as-code
copyartifact
customizable-header
dependency-track
dimensionsscm
ec2
github
gitlab-branch-source
gradle
jenkinslint
localization-support
matrix-auth
miniorange-saml-sp
miniorange-two-factor
opentelemetry
performance-signature-dynatrace
pipeline-input-step
pipeline-maven
pipeline-stage-view
pull-request-monitoring
role-strategy
spotinst
ssh-agents
sysdig-secure
sysdig-secure
xcode

MarkEWaite · 2024-05-20T15:07:32Z

Thanks @mawinter69 ! Is it valid CSS to refer to a CSS class that does not exist? For example, the branch-api plugin today has a line:

<div class="alert alert-warning" role="alert">

Could that be extended to:

<div class="alert jenkins-alert alert-warning" role="alert">

and still be usable on Jenkins versions that don't yet include the new CSS classes?

mawinter69 · 2024-05-20T15:37:58Z

Thanks @mawinter69 ! Is it valid CSS to refer to a CSS class that does not exist?

That would be valid (Jenkins is using all over the place classes without a corresponding css definition to be able to select things in queries). Though only by removing alert and alert-warning there is no interference with bootstrap. And for not breaking plugins that don't have the new classes, the old definitions are still there.

<div class="alert alert-warning" role="alert">

that should become <div class="alert jenkins-alert alert-warning jenkins-alert-warning" role="alert">

…insci#9254) * [JENKINS-73114] avoid conflict of css classes with bootstrap jenkins and bootstrap both have definitions for alert and alert-warning, alert-info, alert-danger. Bootstrap css definitions are included when e.g. the warning-ng plugin is installed and a job is configured to scan for warnings. The css classes from bootstrap are then chosen instead of the ones for Jenkins for things like the admin monitors. To make things umambiguous, add additional classes prefixed with `jenkins-` and make use of them in code. Keep the old definitions for backward compatibility with plugins. Followup changes are needed in plugins (Mostly those that define admin monitors) and the design-library * fix test (cherry picked from commit 90a6ac5)

mawinter69 changed the title ~~[JENKINS-73114] avoid conflict of css classes with bootstrap~~ [JENKINS-73114] avoid conflicts of css classes with bootstrap May 10, 2024

mawinter69 changed the title ~~[JENKINS-73114] avoid conflicts of css classes with bootstrap~~ [JENKINS-73114] avoid conflicts with css classes from bootstrap May 10, 2024

mawinter69 force-pushed the JENKINS-73114 branch from 3502cdc to 282ed72 Compare May 10, 2024 13:30

mawinter69 force-pushed the JENKINS-73114 branch from 282ed72 to cae662b Compare May 10, 2024 14:01

fix test

4f74a62

janfaracik approved these changes May 12, 2024

View reviewed changes

NotMyFault added the bug For changelog: Minor bug. Will be listed after features label May 12, 2024

NotMyFault approved these changes May 12, 2024

View reviewed changes

comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label May 12, 2024

mawinter69 mentioned this pull request May 18, 2024

update banners to use new css classes jenkinsci/design-library-plugin#327

Merged

NotMyFault merged commit 90a6ac5 into jenkinsci:master May 18, 2024
17 checks passed

MarkEWaite added the developer Changes which impact plugin developers label May 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-73114] avoid conflicts with css classes from bootstrap #9254

[JENKINS-73114] avoid conflicts with css classes from bootstrap #9254

mawinter69 commented May 10, 2024 •

edited by MarkEWaite

Submitter checklist

Maintainer checklist

NotMyFault left a comment

timja commented May 18, 2024

mawinter69 commented May 18, 2024

MarkEWaite commented May 19, 2024

mawinter69 commented May 20, 2024

mawinter69 commented May 20, 2024 •

edited by MarkEWaite

MarkEWaite commented May 20, 2024

mawinter69 commented May 20, 2024 •

edited

[JENKINS-73114] avoid conflicts with css classes from bootstrap #9254

[JENKINS-73114] avoid conflicts with css classes from bootstrap #9254

Conversation

mawinter69 commented May 10, 2024 • edited by MarkEWaite

Testing done

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

NotMyFault left a comment

Choose a reason for hiding this comment

timja commented May 18, 2024

mawinter69 commented May 18, 2024

MarkEWaite commented May 19, 2024

mawinter69 commented May 20, 2024

mawinter69 commented May 20, 2024 • edited by MarkEWaite

MarkEWaite commented May 20, 2024

mawinter69 commented May 20, 2024 • edited

mawinter69 commented May 10, 2024 •

edited by MarkEWaite

mawinter69 commented May 20, 2024 •

edited by MarkEWaite

mawinter69 commented May 20, 2024 •

edited