Skip to content
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

Adding green and yellow theme colors, formatting edits for readability & consistency #52

Merged
merged 2 commits into from
Sep 27, 2022

Conversation

tkensiski
Copy link

@tkensiski tkensiski commented Sep 15, 2022

Added a green and yellow themes
Also cleaned up some unused imports and reformatted the css a bit for better readability

Issue: #53

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Note
I think I got everything setup for these, happy to modify as needed.
The readability is subjective I understand but consistency is not so thats what I went for.
Consistency was picked based on the first occurrence where each color was on its own line, this read nice and was easier to maintain in my opinion so I used that format for everywhere else. (https://github.com/jenkinsci/material-theme-plugin/blob/master/src/main/webapp/material-theme.css#L113-L116)

We needed these colors for our instances of jenkins that we previously used afonsof/jenkins-material-theme for; which this is based off of! Colors were taken from there, secondary colors are 75% opacity of the primary color.

…ormatting the css files a bit for better readability with the extra colors being added
@timbrown5
Copy link
Contributor

Theme picker isn't working for me locally:
Screenshot 2022-09-16 at 12 00 09
Not sure why 😕

@timbrown5
Copy link
Contributor

diff --git a/src/main/java/io/jenkins/plugins/materialtheme/MaterialThemeRootAction.java b/src/main/java/io/jenkins/plugins/materialtheme/MaterialThemeRootAction.java
index 48e8ba6..ae9554c 100644
--- a/src/main/java/io/jenkins/plugins/materialtheme/MaterialThemeRootAction.java
+++ b/src/main/java/io/jenkins/plugins/materialtheme/MaterialThemeRootAction.java
@@ -17,6 +17,8 @@ import static io.jenkins.plugins.materialtheme.MaterialIndigoThemeManagerFactory
 import static io.jenkins.plugins.materialtheme.MaterialRedThemeManagerFactory.MATERIAL_RED_CSS;
 import static io.jenkins.plugins.materialtheme.MaterialGreyThemeManagerFactory.MATERIAL_GREY_CSS;
 import static io.jenkins.plugins.materialtheme.MaterialLightBlueThemeManagerFactory.MATERIAL_LIGHT_BLUE_CSS;
+import static io.jenkins.plugins.materialtheme.MaterialYellowThemeManagerFactory.MATERIAL_YELLOW_CSS;
+import static io.jenkins.plugins.materialtheme.MaterialGreenThemeManagerFactory.MATERIAL_GREEN_CSS;
 
 @Extension
 public class MaterialThemeRootAction implements UnprotectedRootAction {
@@ -41,7 +43,7 @@ public class MaterialThemeRootAction implements UnprotectedRootAction {
         if (cssFile.startsWith("/")) {
             cssFile = cssFile.substring(1);
         }
-        if (!Arrays.asList(BASE_CSS, MATERIAL_INDIGO_CSS, MATERIAL_RED_CSS, MATERIAL_GREY_CSS, MATERIAL_LIGHT_BLUE_CSS, CUSTOMISED_CSS)
+        if (!Arrays.asList(BASE_CSS, MATERIAL_INDIGO_CSS, MATERIAL_RED_CSS, MATERIAL_GREY_CSS, MATERIAL_LIGHT_BLUE_CSS, MATERIAL_YELLOW_CSS, MATERIAL_GREEN_CSS, CUSTOMISED_CSS)
                 .contains(cssFile)) {
             rsp.sendError(404);
             return;

I think you need to add your CSS files to the root action as above.

Copy link
Contributor

@timbrown5 timbrown5 left a comment

Choose a reason for hiding this comment

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

I think you need to add the new CSS items to the root action, see:
#52 (comment)

@tkensiski
Copy link
Author

tkensiski commented Sep 20, 2022

I think you need to add the new CSS items to the root action, see: #52 (comment)

Woops sorry about that, I think that should do it

Heres a screenshot too!
Screen Shot 2022-09-20 at 5 19 54 PM

@timbrown5 timbrown5 merged commit 6121925 into jenkinsci:master Sep 27, 2022
@tkensiski
Copy link
Author

@timbrown5 looks like something went wrong in the release process and its not available to the jenkins plugin manager even tho github is showing the correct release version.

@timbrown5
Copy link
Contributor

timbrown5 commented Oct 1, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants