-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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 the appearance of the plugin manager #5916
Conversation
⬇️ Discussion around this would be good ⬇️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bom testing has passed: jenkinsci/bom#756
ATH was tested in jenkinsci/acceptance-test-harness#716.
and issues are resolved in jenkinsci/acceptance-test-harness#718
I think we're ready to ship this.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding an editorconfig is great, but please add the root = true
, so that editor will stop traversing all directories.
Co-authored-by: Joseph Petersen <[email protected]>
@timja I have no further input. I do not think I can approve the frontend part of it 😅 At least it is looking great, so keep up the good work! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, not much has changed since my last review. The new UI looks much better in most places. I think the deprecated banners are still too close to the text, but that was a problem before.
One thing that I really don't like is the position of the search bar. It is far too wide in the upper right corner (where it looks unrelated) and a lot of users will not find them when opening the new page the first time. It already is not ideal in the current UI (where it is above the tabs rather than right above the table), but now I think the usability is getting worse.
But anyway, we can improve that in followup PRs.
This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback. Thanks! |
I created a followup Jira so the remaining review comments will not be forgotten: https://issues.jenkins.io/browse/JENKINS-67366. |
Awesome! I've got a follow up branch in the works to address a lot of your comments - thanks :) |
can you create an issue on https://issues.jenkins.io component core please and link here? so that someone can take a look when they have time |
@janfaracik Causes JENKINS-68277. |
Causes JENKINS-68291. |
What's the purpose of editorconfig when the most important JS file in the project isn't following this to begin with? I currently have to fight against editorconfig on every single line I edit in |
Filed #6863 |
This editorconfig continues to be incredibly annoying, since not all Jelly files are currently indented using 2 spaces. |
📸 Screenshots
Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgradeDesired reviewers
@mention
Maintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are correctupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate
to be considered (see query).