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

Update content and spacing, add link to get more themes #152

Merged
merged 5 commits into from
Dec 1, 2022

Conversation

janfaracik
Copy link
Contributor

Before
image

After
image

What's changed

  • Fixed spacing
  • Changed label 'Built-In Themes' to 'Themes'
  • Added a link to get more themes
  • Removed the help file for the checkbox - the checkbox text seems clear enough as is

Would be good to get some thoughts - thanks :)

@janfaracik janfaracik requested a review from a team as a code owner November 30, 2022 21:31
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:st="jelly:stapler" xmlns:th="/lib/theme-manager">
<f:section title="${%Built-in Themes}">
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:l="/lib/layout" xmlns:th="/lib/theme-manager">
<f:section title="${%Themes}">
Copy link
Member

Choose a reason for hiding this comment

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

the reason this was called Built-in Themes was to stop the double themes label

image

You could probably fix that by making the radio label visually hidden or call it something like select a theme

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, changed it to 'Select theme' 👍

@timja timja added the enhancement New feature or request label Dec 1, 2022
@timja
Copy link
Member

timja commented Dec 1, 2022

Not sure what's going on with the build failure here

@timja
Copy link
Member

timja commented Dec 1, 2022

I can't reproduce it with:

mvn --batch-mode --show-version --errors --no-transfer-progress --update-snapshots -Dmaven.test.failure.ignore -Dspotbugs.failOnError=false -Dcheckstyle.failOnViolation=false -Dcheckstyle.failsOnError=false -Penable-jacoco help:evaluate -Dexpression=changelist  clean install

same java and maven version

@timja
Copy link
Member

timja commented Dec 1, 2022

Looks like https://bugs.openjdk.org/browse/JDK-8297889 fixed in 11.0.18

@timja timja merged commit 08eae65 into jenkinsci:master Dec 1, 2022
@janfaracik janfaracik deleted the small-cleanup branch December 1, 2022 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants