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

Include version number in firstrunwizard #563

Merged
merged 1 commit into from
Jul 20, 2021

Conversation

szaimen
Copy link
Collaborator

@szaimen szaimen commented Jul 16, 2021

Fix #82

Before After
image image

Signed-off-by: szaimen [email protected]

@szaimen szaimen added this to the Nextcloud 23 milestone Jul 16, 2021
@szaimen szaimen force-pushed the enh/82/incloude-version-number branch 2 times, most recently from 3aac6b4 to 007836c Compare July 16, 2021 00:16
@szaimen szaimen marked this pull request as ready for review July 16, 2021 00:29
@juliusknorr
Copy link
Member

Looks good code wise but please have a quick look @nextcloud/designers :)

@szaimen szaimen force-pushed the enh/82/incloude-version-number branch from e2dcefd to 6c68614 Compare July 16, 2021 10:48
@@ -50,5 +50,8 @@
</ul>

<p class="details-link"><a href="<?php p($theme->getBaseUrl()); ?>" target="_blank" rel="noreferrer noopener"><?php p($l->t('Learn more about %s', $theme->getName())); ?></a></p>
<div style="text-align: center;">
<h3><?php p($l->t('This Nextcloud is on version %s', [ \OC_Util::getVersionString() ])); ?></h3>
Copy link
Member

@tcitworld tcitworld Jul 16, 2021

Choose a reason for hiding this comment

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

As the rest of the steps use the theme name a lot I would suggest something more like :
$theme->getName() is using Nextcloud version %s

If the theme name is set, of course.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm... I would actually rather do that in a follow up to this PR...
@juliushaertl is that fine?

Copy link
Member

@jancborchardt jancborchardt Jul 19, 2021

Choose a reason for hiding this comment

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

If we do show the version here, it should be rather short and inconspicuous. :) And it’s not a h3, a <p> with color: var(--text-maxcontrast); would be better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done
This is how it looks now:
image

Signed-off-by: szaimen <[email protected]>
Co-Authored-By: Julius Härtl <[email protected]>
@szaimen szaimen force-pushed the enh/82/incloude-version-number branch from 6c68614 to c9e70b6 Compare July 19, 2021 16:34
@szaimen szaimen requested review from jancborchardt and removed request for jancborchardt July 19, 2021 16:38
@juliusknorr juliusknorr merged commit a3896b2 into master Jul 20, 2021
@juliusknorr juliusknorr deleted the enh/82/incloude-version-number branch July 20, 2021 06:38
@szaimen
Copy link
Collaborator Author

szaimen commented Jul 20, 2021

/backport to stable22

@szaimen
Copy link
Collaborator Author

szaimen commented Jul 20, 2021

/backport to stable21

@szaimen
Copy link
Collaborator Author

szaimen commented Jul 20, 2021

/backport to stable20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add version number to the first run wizard?
5 participants