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

Refine content and appearance of the user account screen #9521

Merged
merged 13 commits into from
Sep 16, 2024

Conversation

janfaracik
Copy link
Contributor

@janfaracik janfaracik commented Aug 3, 2024

This pull request updates the content and minor styling of the user account screen to make the content a little cleaner and inline. This is part of an ongoing effort to simplify Jenkins' many forms.

Changes

  • Reduced usages of the❓button in the form, preferably if help guidance is short, snappy and useful it should be displayed inline
  • Refined content for 'Full name', 'Description', 'Console URL Provider' and 'User Defined Time Zone'
  • UserPropertyDescriptor now has a getDescription() method that can be overridden - these descriptions will show on pages where supported
  • Wrapped the form in the jenkins-form class to fix sizing
  • Grouped the first two controls (name and description) in a section so that they display with a horizontal divider below

View comparison


Before

354807802-40fa4474-379b-4867-b369-efacdca36d00


After

354807736-1df0c585-2451-4a3d-bc48-989742f57296

Testing done

  • User config page works as before

Proposed changelog entries

  • Refine content and appearance of the user account screen.

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

@comment-ops-bot comment-ops-bot bot added the web-ui The PR includes WebUI changes which may need special expertise label Aug 3, 2024
@timja timja added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Aug 3, 2024
@timja
Copy link
Member

timja commented Aug 3, 2024

FYI something is wrong with your screenshots maybe transparency? in dark mode they are really messed up, and even in light mode if I open in a new tab they don't render right for me

Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>
@janfaracik
Copy link
Contributor Author

FYI something is wrong with your screenshots maybe transparency? in dark mode they are really messed up, and even in light mode if I open in a new tab they don't render right for me

Should be fixed now, thanks!

@NotMyFault NotMyFault requested a review from timja August 13, 2024 05:33
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.

It is unusual for a descriptor to always have a single f:section, this might be the only form where that's actually the case. Given limited applicability (a single Descriptor here?) it seems unnecessary to extend Descriptor for this use case. Please demonstrate this is useful well beyond the descriptors and forms shown in this PR.

core/src/main/java/hudson/model/Descriptor.java Outdated Show resolved Hide resolved
@janfaracik
Copy link
Contributor Author

It is unusual for a descriptor to always have a single f:section, this might be the only form where that's actually the case. Given limited applicability (a single Descriptor here?) it seems unnecessary to extend Descriptor for this use case. Please demonstrate this is useful well beyond the descriptors and forms shown in this PR.

That's completely fair, I've moved it to UserPropertyDescriptor

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, tested and all appears sensible.

@timja timja requested a review from a team September 13, 2024 08:07
@timja
Copy link
Member

timja commented Sep 13, 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 Sep 13, 2024
@timja timja merged commit 2d2adef into jenkinsci:master Sep 16, 2024
16 checks passed
@janfaracik janfaracik deleted the improve-user-settings branch September 18, 2024 17:46
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.

4 participants