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

Increase max-height on button in welcome email template #20447

Merged
merged 2 commits into from
Apr 17, 2020

Conversation

liamjack
Copy link
Contributor

Fixes #13287 implementing @fuanegua's fix proposal (#13287 (comment)) by increasing the max-height of the button to 60px from 40px.

Before:

Short Nextcloud instance name

before_short

Long Nextcloud instance name

before_long

After

Short Nextcloud instance name

after_short

Long Nextcloud instance name

after_long

…template

Signed-off-by: Liam JACK <liamjack@users.noreply.github.com>
@gary-kim gary-kim added 3. to review Waiting for reviews bug design Design, UI, UX, etc. labels Apr 12, 2020
@gary-kim gary-kim added this to the Nextcloud 19 milestone Apr 12, 2020
@kesselb
Copy link
Contributor

kesselb commented Apr 12, 2020

Would it be possible to have the same size for the install client button?

Thanks for attaching screenshots. That makes reviewing much easier 👍

@liamjack
Copy link
Contributor Author

I've had a go, but I'm unable to make the "install client" button have the same height as the "go to" button, sorry !

@rullzer rullzer mentioned this pull request Apr 13, 2020
59 tasks
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Looks good, thanks a lot for your contribution @liamjack! :)

@jancborchardt
Copy link
Member

@liamjack @kesselb @fuanegua could you also review Fix design and layout of notification mails #20380 which is very related? Thanks!

This was referenced Apr 15, 2020
@MorrisJobke MorrisJobke changed the title Fix #13287 - Increase max-height on button in welcome email template Increase max-height on button in welcome email template Apr 17, 2020
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Copy link
Member

I pushed a fix to the two missing test cases. This should be fine then.

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Works 👍

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 17, 2020
@skjnldsv skjnldsv merged commit 36f9ad3 into nextcloud:master Apr 17, 2020
@welcome
Copy link

welcome bot commented Apr 17, 2020

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@skjnldsv
Copy link
Member

AH, I might have been a little too fast

2) OCA\Settings\Tests\Mailer\NewUserMailHelperTest::testGenerateTemplateWithoutPasswordResetToken
391 | Failed asserting that two strings are equal.
392 | --- Expected
393 | +++ Actual
394 | @@ @@
395 | Install Client: https://nextcloud.com/install/#install-clients\n
396 | \n
397 | \n
398 | ---\n
399 | -TestCloud -\n
400 | +-- \n
401 | +TestCloud - \n
402 | This is an automatically sent email, please do not reply.'
403 |  
404 | /drone/src/apps/settings/tests/Mailer/NewUserMailHelperTest.php:624
405 |  
406 | 3) OCA\Settings\Tests\Mailer\NewUserMailHelperTest::testGenerateTemplateWithoutUserId
407 | Failed asserting that two strings are equal.
408 | --- Expected
409 | +++ Actual
410 | @@ @@
411 | Install Client: https://nextcloud.com/install/#install-clients\n
412 | \n
413 | \n
414 | ---\n
415 | -TestCloud -\n
416 | +-- \n
417 | +TestCloud - \n
418 | This is an automatically sent email, please do not reply.'
419 |  
420 | /drone/src/apps/settings/tests/Mailer/NewUserMailHelperTest.php:848
421 |  

Let's see tests

@rullzer
Copy link
Member

rullzer commented Apr 17, 2020

Fix for the tests in #20540

@liamjack liamjack deleted the fix/13287/welcome_email branch April 18, 2020 09:09
@rullzer
Copy link
Member

rullzer commented Jun 12, 2020

/backport to stable18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug design Design, UI, UX, etc. feature: emails
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Set your password" text on button in welcome mail out of bounds
7 participants