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

Modernise project relationship page #9461

Merged
merged 4 commits into from
Aug 10, 2024

Conversation

timja
Copy link
Member

@timja timja commented Jul 13, 2024

This was the only usage in core of the long deprecated f:editableComboBox

Editable drop-down combo box. Deprecated as of 1.356. Use f:combobox and databinding instead.

Few more around outside of core though: https://github.com/search?q=org%3Ajenkinsci%20editableComboBox&type=code

This cleans up the styling so the page doesn't look so out of place
I don't fully understand the page and so I haven't improved the rendering of the results, but the inputs and general look and feel are improved

Before:

image

After:

image

We could possibly get rid of the help sub-page but the content seemed a bit much so I just copied some of it over to explain the page a bit.

Testing done

see screenshots above, I've also tested with missing parameters.

To setup the test environment I used an upstream and downstream project, created a file in the upstream, archived it (and enabled fingerprinting), enabled the copy artifact setting on the upstream job so that downstream could copy it and then copied the artifact in the downstream job recording the fingerprint (using copy artifact plugin)

Proposed changelog entries

  • Modernize project relationship page.

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@mention

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

Maintainer checklist

core/src/main/java/jenkins/model/Jenkins.java Show resolved Hide resolved
@@ -0,0 +1,27 @@
# The MIT License
#
# Bulgarian translation: Copyright (c) 2024, Tim Jacomb
Copy link
Member

Choose a reason for hiding this comment

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

Seems out of place, this is the English file?

Copy link
Member Author

Choose a reason for hiding this comment

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

whyyyy is that there, I just copied the header from that file the only one with that -.-

@timja timja requested a review from daniel-beck July 13, 2024 22:38
@timja timja added rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted web-ui The PR includes WebUI changes which may need special expertise labels Jul 13, 2024
@basil
Copy link
Member

basil commented Jul 17, 2024

I liked the infix operator used on the old page because it usually denotes a causal relationship which is the case between an upstream project and downstream project. Surely the fact that GitHub uses this symbol in their compare view between upstream and downstream repositories/branches is evidence that it is sufficiently modern.

@timja
Copy link
Member Author

timja commented Jul 17, 2024

Yes although it doesn't really fit with the maintained form components. I doubt this page is used much and don't think special casing it is worth it.

The primary motivation for this PR is to remove the long deprecated component.

@timja
Copy link
Member Author

timja commented Jul 31, 2024

/label ready-for-merge


This PR is now ready for merge, after the branch is unlocked, we will merge it if there's no negative feedback.

Thanks!

@comment-ops-bot 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 Jul 31, 2024
@timja timja merged commit 824c868 into jenkinsci:master Aug 10, 2024
16 checks passed
@timja timja deleted the project-relationship-modernise branch August 10, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted web-ui The PR includes WebUI changes which may need special expertise
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants