Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-62739

Do not use Bootstrap style in Advisor admin page

    • cloudbees-jenkins-advisor-3.2.0

      Reported in the dark-theme plugin ( https://github.com/jenkinsci/dark-theme-plugin/issues/79 ) by timja and confirmed by fqueiruga using Bootstrap in the admin panel is causing various rendering issue (Bootstrap causes the specific pages where it's used to have a different font and basic theme than the rest of the UI.)

      AFAICS Bootstrap is used for the alert boxes used in the admin page (success, info, warning, error) and Bootstrap was imported to match what was done in the "Manage Jenkins" screen for monitors

      It seems possible to do without it ... let's try

          [JENKINS-62739] Do not use Bootstrap style in Advisor admin page

           Maybe 2.204 can be a good new baseline target. There would still be some work needed to make it forward-compatible with theming but nothing to worry about.

          FWIW bootstrap was removed from the /manage page on 2.226.

           

          Félix Queiruga Balado added a comment -  Maybe 2.204 can be a good new baseline target. There would still be some work needed to make it forward-compatible with theming but nothing to worry about. FWIW bootstrap was removed from the /manage page on 2.226.  

          Tim Jacomb added a comment -

          The minimum version is 2.200

          https://github.com/jenkinsci/jenkins/pull/4276
          https://issues.jenkins-ci.org/browse/JENKINS-59684

          alert-danger, alert-info, alert-warning

          Tim Jacomb added a comment - The minimum version is 2.200 https://github.com/jenkinsci/jenkins/pull/4276 https://issues.jenkins-ci.org/browse/JENKINS-59684 alert-danger, alert-info, alert-warning

          fqueiruga it is fine for me to upgrade the requirement to 2.204 LTS ( Released 6 months ago - 2019-12-18 )

          Can you help me for backporting the CSS?

          Otherwise if you have any other / better approach to make visible the alert messages feel free to propose something else.

          A part of the screenshots showing the usage of these alert boxes are visible in https://issues.jenkins-ci.org/browse/JENKINS-59648 and https://github.com/jenkinsci/cloudbees-jenkins-advisor-plugin/pull/37

          Arnaud Héritier added a comment - fqueiruga it is fine for me to upgrade the requirement to 2.204 LTS ( Released 6 months ago - 2019-12-18 ) Can you help me for backporting the CSS? Otherwise if you have any other / better approach to make visible the alert messages feel free to propose something else. A part of the screenshots showing the usage of these alert boxes are visible in  https://issues.jenkins-ci.org/browse/JENKINS-59648  and  https://github.com/jenkinsci/cloudbees-jenkins-advisor-plugin/pull/37

          Tim Jacomb added a comment -

          There's no need to backport if you use 2.204, it was added in 2.200

          Tim Jacomb added a comment - There's no need to backport if you use 2.204, it was added in 2.200

          Félix Queiruga Balado added a comment - - edited

          Now that I think of it maybe no backport is needed. When it runs on a version of Jenkins > 2.240 it will automatically pick up theming. Is that right timja? Maybe I'm overcomplicating things.

          Edit: see Tim's comment above

          Félix Queiruga Balado added a comment - - edited Now that I think of it maybe no backport is needed. When it runs on a version of Jenkins > 2.240 it will automatically pick up theming. Is that right timja ? Maybe I'm overcomplicating things. Edit: see Tim's comment above

          timja fqueiruga ok I will try to upgrade and test without bootstrap.

          I will miss alert-success AFAICS but I can replace it with alert-info

          Arnaud Héritier added a comment - timja fqueiruga ok I will try to upgrade and test without bootstrap. I will miss alert-success AFAICS but I can replace it with alert-info

          Félix Queiruga Balado added a comment - - edited

          You should be able to backport alert-success with the following CSS:

          .alert-success {
            background-color: #d4edda;
            background-color: var(--alert-success-bg-color);
            border-color: #c3e6cb;
            border-color: var(--alert-success-border-color);
            color: #138347;
            color: var(--alert-success-color);  
          }
          

          Color codes may not be final but for the purposes of testing it out will do.

          This one should also be added upstream IMO. There is already a success version for the form "Saved" banner.

          Edit: I have created a ticket for this https://issues.jenkins-ci.org/browse/JENKINS-62747 

          Félix Queiruga Balado added a comment - - edited You should be able to backport alert-success with the following CSS: .alert-success { background-color: #d4edda; background-color: var (--alert-success-bg-color); border-color: #c3e6cb; border-color: var (--alert-success-border-color); color: #138347; color: var (--alert-success-color); } Color codes may not be final but for the purposes of testing it out will do. This one should also be added upstream IMO. There is already a success version for the form "Saved" banner. Edit: I have created a ticket for this  https://issues.jenkins-ci.org/browse/JENKINS-62747  

          It would also help if you could wrap everything within <l:main-panel> here https://github.com/jenkinsci/cloudbees-jenkins-advisor-plugin/blob/a73267a742e3d0d629a46b507433a1e38e27b345/src/main/resources/com/cloudbees/jenkins/plugins/advisor/AdvisorGlobalConfiguration/index.jelly#L20 inside a div with a CSS class like .jenkins-health-advisor. This way it would be possible to namespace CSS to only work within that part of the page.

          Félix Queiruga Balado added a comment - It would also help if you could wrap everything within <l:main-panel> here https://github.com/jenkinsci/cloudbees-jenkins-advisor-plugin/blob/a73267a742e3d0d629a46b507433a1e38e27b345/src/main/resources/com/cloudbees/jenkins/plugins/advisor/AdvisorGlobalConfiguration/index.jelly#L20  inside a div with a CSS class like .jenkins-health-advisor . This way it would be possible to namespace CSS to only work within that part of the page.

           ok I will try. Thx for these feedbacks

          Arnaud Héritier added a comment -  ok I will try. Thx for these feedbacks

          Arnaud Héritier added a comment - PR  https://github.com/jenkinsci/cloudbees-jenkins-advisor-plugin/pull/60 To be released as 3.2.0 when validated by fqueiruga

            aheritier Arnaud Héritier
            aheritier Arnaud Héritier
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: