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

Use Notice component for views lacking jobs #9724

Merged
merged 4 commits into from
Sep 15, 2024

Conversation

janfaracik
Copy link
Contributor

@janfaracik janfaracik commented Sep 12, 2024

Small one to use the <l:notice /> component for views lacking jobs. It's more visual, featuring an icon and bolder text.

Before
image

After
image

Testing done

  • Notice displays as expected

Proposed changelog entries

  • Use Notice component for views lacking jobs.

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@jenkinsci/sig-ux

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

Maintainer checklist

@janfaracik
Copy link
Contributor Author

/label web-ui,rfe

/reviewer @jenkinsci/sig-ux

@comment-ops-bot comment-ops-bot bot added web-ui The PR includes WebUI changes which may need special expertise rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted labels Sep 12, 2024
@comment-ops-bot comment-ops-bot bot requested a review from a team September 12, 2024 13:44
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Looks good in my interactive testing. I checked that the message was correctly displayed both when the user has permission and when they do not have permission.

screencapture-rhel-8-a-markwaite-net-8080-jenkins-view-empty-view-2024-09-12-18_47_58-edit

@MarkEWaite MarkEWaite self-assigned this Sep 13, 2024
@timja timja added the needs-security-review Awaiting review by a security team member label Sep 13, 2024
@timja timja requested a review from a team September 13, 2024 07:57
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.

LGTM

@daniel-beck daniel-beck self-requested a review September 13, 2024 09:53
Copy link
Member

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

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

Thanks for adapting. Works well in limited testing. The change to invokeBody makes it clear that it supports HTML as well.

@daniel-beck daniel-beck added security-approved @jenkinsci/core-security-review reviewed this PR for security issues and removed needs-security-review Awaiting review by a security team member labels Sep 14, 2024
@MarkEWaite
Copy link
Contributor

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.

/label ready-for-merge

@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 Sep 14, 2024
@MarkEWaite MarkEWaite merged commit 239ced0 into jenkinsci:master Sep 15, 2024
16 checks passed
@janfaracik janfaracik deleted the new-view-notice branch September 16, 2024 05:59
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 security-approved @jenkinsci/core-security-review reviewed this PR for security issues 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