-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Update UI Screenshots for In-process Script Approval and Change System Time Zone in Jenkins Documentation #7559
base: master
Are you sure you want to change the base?
Conversation
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.
Please narrow the new screenshots so that they do not sacrifice readability for users on more narrow screens. Increasing the width of the screenshot by 2x will make it much more difficult for readers of narrower screens
My comment above is wrong. The wider screenshot does not sacrifice readability in any significant way. The markup explicitly sets the image width on the HTML page to 800, so it is the same for wide or narrow screens. For visual comparison: |
Thank you for your feedback. I had tested the changes locally using Docker and felt that the updated screenshots were appropriate. After your earlier comment yesterday, I was planning to make adjustments by zooming out and taking new screenshots to decrease the height and width. However, after your latest comment on the preview, I'm now unsure if further changes are necessary. Could you please clarify if any additional adjustments are still needed, or if the current screenshots are acceptable as they are? Thanks again for your time and guidance! |
I compared each of the screenshots before and after and I believe that one of the larger screenshots is much more difficult to read than it was before: content/doc/book/resources/managing/unchecked-groovy-sandbox-on-pipeline.png Comparisons (will need to scroll downward to find the image): The wider images do not seem to affect the others nearly as much as that one is affected. |
Hi @gitclonegaurang, I was looking at the files uploaded and found that they could be compressed a bit further than what they are currently at. In doing so, these two images were reduced by over over 50% and save some space. If you are already running the images through compression, it may be worth checking to see if they can be compressed further without losing quality. This image is a bit larger than what is currently shown and interrupts the text a bit. If possible, would you be able to capture this image on a smaller window so that the syntax box is not as wide? It should re-size dynamically so that the text remains the same size but the overall box should reduce in size and allow for a more fitting screenshot. |
Hi @kmartens27, Thank you for the detailed feedback! I'll make these adjustments and update the PR as soon as possible. Thanks again for your review and guidance! |
@gitclonegaurang I just noticed that you are making these changes from the |
Hi @gitclonegaurang just to share a heads up, another user has completed the screenshot update for the Time Zone documentation, so you can remove those images from this pul request. However, the in-process script approval work should still be updated as previously suggested. Let me know if you have any issues or questions on this! |
Alright |
Hey @kmartens27 I just observed that the screenshot update for change system zone is not yet updated as mentioned in issue #7527 as upon clicking on the Change System Time Zone the image is same as it was at the time of this issue raised . Do let me know if I have to update the same also. For in-process script approval I will make the changes asap. |
Hi @gitclonegaurang sorry for the confusion on my part. The link I was thinking of was https://www.jenkins.io/doc/book/using/change-time-zone/ which would be different for what you're working on in this PR. You are correct, please proceed with what you had been doing in the first place. They're very similar URLs and documentation so that's on me. Sorry for the confusion! |
No problem I will work on them |
Updated outdated UI screenshots for the following pages:
Screenshots have been replaced with more recent images reflecting the latest UI changes.
Images modified:
change-system-timezone-user-defined-timezone-2.png
change-system-timezone-user-defined-timezone.png
inprocess-script-approval-pipeline.png
manage-inprocess-script-approval.png
unchecked-groovy-sandbox-on-pipeline.png