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

Update appearance of 'Jenkins is starting' pages #9707

Merged
merged 15 commits into from
Sep 13, 2024

Conversation

janfaracik
Copy link
Contributor

@janfaracik janfaracik commented Sep 8, 2024

This PR updates the appearance of the 'Jenkins is starting' pages. The pages are now slightly more padded, content has been refined, use the <l:spinner/> component (rather than a custom one) and have a soft animation on load.

Booting

Before
image

After
image

Safe restart

Before
image

After
image

Restart

Before
image

After
image

⚠️ Unsure of how to access core/src/main/resources/hudson/lifecycle/WindowsInstallerLink/_restart.jelly

Testing done

  • Pages appear as expected

Proposed changelog entries

  • Update appearance of 'Jenkins is starting' pages.

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 Sep 8, 2024
@timja
Copy link
Member

timja commented Sep 8, 2024

You can do:

/label web-ui,rfe
/reviewer @jenkinsci/sig-ux

@comment-ops-bot comment-ops-bot bot added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Sep 8, 2024
Copy link

I wasn't able to request review for the following reviewer(s): jenkinsci/sig-ux

Check that the reviewer is spelt right and try again.

@timja
Copy link
Member

timja commented Sep 8, 2024

/reviewer @jenkinsci/sig-ux

@comment-ops-bot comment-ops-bot bot requested a review from a team September 8, 2024 12:58
@@ -1,5 +1,5 @@
Please\ wait\ while\ Jenkins\ is\ restarting=Jenkins wird neu gestartet. Bitte warten.
Copy link
Member

Choose a reason for hiding this comment

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

When changing the English text we should update / remove translations.

e.g. here I think you would drop the Bitte warten

Copy link
Member

Choose a reason for hiding this comment

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

I can confirm, it should be "Jenkins wird neu gestartet"

Copy link
Member

@timja timja Sep 10, 2024

Choose a reason for hiding this comment

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

Suggested change
Please\ wait\ while\ Jenkins\ is\ restarting=Jenkins wird neu gestartet. Bitte warten.
Please\ wait\ while\ Jenkins\ is\ restarting=Jenkins wird neu gestartet

Copy link
Member

Choose a reason for hiding this comment

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

Should the "." be part of the text or not?

@@ -21,5 +21,5 @@
# THE SOFTWARE.

Please\ wait\ while\ Jenkins\ is\ getting\ ready\ to\ work=Por favor aguarde enquanto o Jenkins se prepara para trabalhar
Copy link
Member

Choose a reason for hiding this comment

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

This needs updating still (capturing as outstanding)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, updated the translations that I could, removed those that I couldn't

@janfaracik janfaracik changed the title Updated appearance of 'Jenkins is starting' pages Update appearance of 'Jenkins is starting' pages Sep 9, 2024
timja
timja previously approved these changes Sep 10, 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

@timja timja requested a review from a team September 10, 2024 08:01
@@ -20,5 +20,5 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.

Your\ browser\ will\ reload\ automatically\ when\ Jenkins\ is\ ready.=Din browser vil genindlæse siden automatisk når Jenkins er klar.
Your\ browser\ will\ reload\ automatically\ when\ Jenkins\ is\ ready=Din browser vil genindlæse siden automatisk når Jenkins er klar.
Please\ wait\ while\ Jenkins\ is\ restarting=Vent venligst imens Jenkins genstarter
Copy link
Member

Choose a reason for hiding this comment

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

there's still a number of translations outstanding for this one Please\ wait\ while\ Jenkins\ is\ restarting

Either update or remove translation please

@timja timja dismissed their stale review September 10, 2024 09:23

More translations outstanding

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

@timja
Copy link
Member

timja commented Sep 11, 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 11, 2024
<l:spinner text="${%Please wait while Jenkins is getting ready to work}" />
</h1>
<p class="restarting">
${%Your browser will reload automatically when Jenkins is ready}
Copy link
Member

Choose a reason for hiding this comment

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

Why remove the trailing period? The other line is (or was) a title so didn't need it, but this one isn't.

Copy link
Contributor Author

@janfaracik janfaracik Sep 11, 2024

Choose a reason for hiding this comment

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

Personal preference, I don't personally see the value of them for things like this. Would be a good thing to standardise across Jenkins, as right now many fields include periods, many don't.

For this piece of text in particular, it's centered and I think it appears more visually balanced without the punctuation.

Copy link
Member

Choose a reason for hiding this comment

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

many fields include periods, many don't

Are there many sentences that don't have periods?

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.

Worked well in my interactive testing

@MarkEWaite MarkEWaite self-assigned this Sep 13, 2024
@timja timja merged commit ce3d8c1 into jenkinsci:master Sep 13, 2024
16 checks passed
@timja timja deleted the updated-startup-restart branch September 13, 2024 06:42
@@ -20,5 +20,5 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.

Your\ browser\ will\ reload\ automatically\ when\ Jenkins\ is\ ready.=Din browser vil genindlæse siden automatisk når Jenkins er klar.
Please\ wait\ while\ Jenkins\ is\ restarting=Vent venligst imens Jenkins genstarter
Copy link
Member

Choose a reason for hiding this comment

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

Was this intentional? I don't speak the language but Google Translate says it's now "Please wait Jenkins restarts" which seems not great translated back.

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.

6 participants