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

Display how many users there are on the 'Users' page #9221

Merged
merged 2 commits into from
May 27, 2024

Conversation

janfaracik
Copy link
Contributor

@janfaracik janfaracik commented May 2, 2024

Tiny one to show how many users there are on the 'Users' page. Follows the same pattern as the prototype.

image

Testing done

  • Shows how many users there are
  • Displays 0 when there are no users

Proposed changelog entries

  • Display how many users there are on the 'Users' page

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@jenkinsci/sig-ux

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

Maintainer checklist

@uhafner
Copy link
Member

uhafner commented May 2, 2024

Don't we use badges in other areas for the same thing? When should I use a badge and when this new style? It makes sense to clarify in the design library?

@janfaracik
Copy link
Contributor Author

janfaracik commented May 7, 2024

Don't we use badges in other areas for the same thing? When should I use a badge and when this new style? It makes sense to clarify in the design library?

Opened a PR for the design library here - jenkinsci/design-library-plugin#325.

Gist is show a badge in the side panel for important info/info the user might need to take action on (e.g. updates, test success/failures) and show that in the app bar if that makes sense too. In this instance the number of users isn't important unless you're on this page, so there isn't value in showing it outside of this page as a badge.

@NotMyFault NotMyFault requested a review from a team May 11, 2024 18:40
@NotMyFault NotMyFault 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 May 11, 2024
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, could maybe format the number but may not affect many places:

image

@Kevin-CB Kevin-CB added the security-approved @jenkinsci/core-security-review reviewed this PR for security issues label May 21, 2024
@timja timja requested a review from a team May 21, 2024 14:47
@timja
Copy link
Member

timja commented May 26, 2024

/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!

@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 May 26, 2024
@NotMyFault NotMyFault merged commit 2c02654 into jenkinsci:master May 27, 2024
17 checks passed
@timja timja deleted the users-app-bar-count branch May 27, 2024 21:22
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