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 <select> config appearance #202

Closed
wants to merge 1 commit into from

Conversation

janfaracik
Copy link

This PR updates the <select> config appearance to use the new classes introduced in Jenkins #5923.

Before
image

After
image

Just a heads up that the pre-existing tooltip attribute seemingly doesn't do anything on <option>, tried it on Safari and Chrome.

Cheers.

  • 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
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@olamy
Copy link
Member

olamy commented Feb 21, 2022

this will need to upgrade the pom value <jenkins.version>2.249.1</jenkins.version> to a non LTS version.

@janfaracik
Copy link
Author

this will need to upgrade the pom value <jenkins.version>2.249.1</jenkins.version> to a non LTS version.

Apologies, bit of a novice in this area. Is this still necessary if this PR still works/performs the same way on current LTS Jenkins?

@olamy
Copy link
Member

olamy commented Feb 22, 2022

this will need to upgrade the pom value <jenkins.version>2.249.1</jenkins.version> to a non LTS version.

Apologies, bit of a novice in this area. Is this still necessary if this PR still works/performs the same way on current LTS Jenkins?

no worries.
if this need too much unmaintainable code for a limited period of time not sure it worth it.
there are usually some rules https://www.jenkins.io/doc/developer/plugin-development/choosing-jenkins-baseline/
I can see some plugins are upgrading to some non LTS but depends what others contributors think about this.

@janfaracik
Copy link
Author

this will need to upgrade the pom value <jenkins.version>2.249.1</jenkins.version> to a non LTS version.

Apologies, bit of a novice in this area. Is this still necessary if this PR still works/performs the same way on current LTS Jenkins?

no worries. if this need too much unmaintainable code for a limited period of time not sure it worth it. there are usually some rules https://www.jenkins.io/doc/developer/plugin-development/choosing-jenkins-baseline/ I can see some plugins are upgrading to some non LTS but depends what others contributors think about this.

Any thoughts on this @timja?

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

@janfaracik Jenkins plugins use the version defined in the jenkins.version as the minimum version of Jenkins it's compatible with.

Historically maintainers did not like bumping it too high as people not up to date wouldn't get updates.
That's changed a bit in the last year or two, people seem to have decided that the people updating will get updates and critical fixes can always be backported to the old line.

Basically either you need to make sure it works on old versions of core and new versions.
Either by applying both stylings or version / feature detection

Or bump the minimum version up.

This plugin could certainly do with a minimum version bump but given how trivial it is to keep compatibility probably doesn't need to be done in this PR

</j:forEach>
</select>
<div class="jenkins-select">
<select class="jenkins-select__input" name="durabilityHint">
Copy link
Member

Choose a reason for hiding this comment

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

This change appears to be backwards and forwards compatible

Suggested change
<select class="jenkins-select__input" name="durabilityHint">
<!-- TODO Remove setting-input when baseline above 2.335 -->
<select class="jenkins-select__input setting-input" name="durabilityHint">

@timja
Copy link
Member

timja commented Mar 1, 2022

Thinking aloud is there not a tag built into Jenkins for this?

Why is this writing plain HTML ?

@timja
Copy link
Member

timja commented Mar 1, 2022

see #205

@janfaracik
Copy link
Author

see #205

#205 is infinitely better than mine - good job! I'll close this one.

@janfaracik janfaracik closed this Mar 1, 2022
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.

3 participants