-
-
Notifications
You must be signed in to change notification settings - Fork 9k
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
feat: Add empty state for no old data #10320
Conversation
core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly
Outdated
Show resolved
Hide resolved
core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly
Outdated
Show resolved
Hide resolved
core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly
Outdated
Show resolved
Hide resolved
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.
UI looks good, jelly just needs a little polish
@timja I've made some changes to the Jelly file , please check now |
change looks good but you haven't done https://github.com/jenkinsci/jenkins/pull/10320/files#r1966735160 |
@timja yup , sorry I missed that earlier , done it now. |
core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly
Outdated
Show resolved
Hide resolved
core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly
Outdated
Show resolved
Hide resolved
@timja made the change now , is it fine? |
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.
LGTM
I don't think that the implementation is complete. While looping over the elements of the table it does the check |
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.
Maybe one can already have a flag when the data is collected whether the table would be empty and the hasExtra will be set.
/label web-ui |
@mawinter69 I've made the desired changes as you suggested |
core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly
Outdated
Show resolved
Hide resolved
core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly
Outdated
Show resolved
Hide resolved
Any thoughts on "No old data" as the panel text? "No old data was found" to me sounds like it failed to find old data. |
@janfaracik yes , No old data seems a better fit to me too. Shall I change it? |
yes please |
Done |
core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly
Outdated
Show resolved
Hide resolved
core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly
Outdated
Show resolved
Hide resolved
core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly
Outdated
Show resolved
Hide resolved
Co-authored-by: Kris Stern <[email protected]>
/label ready-for-merge This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback. 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.
LGTM.
Great thanks to all of you for your continuous guidance !! |
Fixes jenkinsci/sig-ux#11.
Added an empty state to Manage Old Data to not display an empty table , instead show No old data was found.
Testing done
Before:


After:
Proposed changelog entries
Proposed changelog category
/label rfe
Proposed upgrade guidelines
N/A
Submitter checklist
@Restricted
or have@since TODO
Javadocs, as appropriate.@Deprecated(since = "TODO")
or@Deprecated(forRemoval = true, since = "TODO")
, if applicable.eval
to ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@timja @janfaracik
Before the changes are marked as
ready-for-merge
:Maintainer checklist
upgrade-guide-needed
label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidate
to be considered (see query).