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

fix margin in readonly mode #8938

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented Feb 4, 2024

when readonly mode is enabled with systemread or extended read permissions, some fields are wrapped in a pre tag. By default pre has a bigger margin-bottom . But in readonly mode we already have a margin in a wrapping div which leads to unnecessary large space after those fields.
So we should remove the margin-bottom for this

Before:
image

After:
image

Testing done

manual testing:

  • install extended read permissions plugin
  • create user and grant systemread permission to it
  • visit system configure page with this user

Proposed changelog entries

  • Remove the extra margin when viewing in read only mode.

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@mention

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

Maintainer checklist

when readonly mode is enabled with systemread or extended read
permissions, some fields are wrapped in a `pre` tag. By default `pre`
has a bigger margin-bottom . But in readonly mode we already have a
margin in a wrapping div which leads to unnecessary large space after
those fields.
So we should remove the margin-bottom for this
@NotMyFault NotMyFault requested a review from a team February 5, 2024 16:10
@NotMyFault NotMyFault added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Feb 5, 2024
Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

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

Thanks!


/label ready-for-merge


This PR is now ready for merge. We will merge it after ~24 hours if there is no negative feedback.
Please see the merge process documentation for more information about the merge process.
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 Feb 6, 2024
@NotMyFault NotMyFault merged commit 90c3a39 into jenkinsci:master Feb 8, 2024
17 checks passed
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants