New layout for welcome email #4244

MorrisJobke · 2017-04-06T18:23:49Z

thanks to @Espina2 for make this nice design
the button says "Set password" if the admin didn't specified a password
if a display name is set for the user it says "Welcome aboard DISPLAYNAME"
see Better designed emails when sharing links and calendar etc #2071

@juliushaertl @Espina2 @jancborchardt @LukasReschke @rullzer @tobiasKaminsky For a first feedback

mention-bot · 2017-04-06T18:23:50Z

@MorrisJobke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jancborchardt, @eppfel and @rullzer to be potential reviewers.

MorrisJobke · 2017-04-07T05:23:57Z

There are now also unit tests for this helper. That checks if a working (nearly the same as the example by @Espina2 ) template gets produced.

MorrisJobke · 2017-04-07T06:01:35Z

@LukasReschke Could you please have a look at the failing unit tests? That would be awesome :)

Espina2 · 2017-04-07T08:43:19Z

@MorrisJobke Nice work!!
The last text after the black logo it should be more greyish since is a secondary info.

LukasReschke · 2017-04-07T09:02:20Z

Tests should be fixed now @MorrisJobke

LukasReschke

Works fine here. With regards to the theming integration I'd recommend:

For the header just use the configured logo of the instance.
For the footer use nothing if a custom logo is configured. If none is configured use the Nextcloud logo.

With regard to the color issue pointed out by @Espina2, what is the hex code that we should use there? :-)

LukasReschke · 2017-04-07T09:06:59Z

lib/private/Mail/IEMailTemplate.php

+	 *
+	 * @param string $logoUrl
+	 */
+	public function addHeader($logoUrl);


I'd say we can remove $logoUrl from the interface. Let that automatically be detected. Also easier because otherwise it's a mess if you in the controller have to detect theming etc…

LukasReschke · 2017-04-07T09:07:12Z

lib/private/Mail/IEMailTemplate.php

+	 * @param string $logoUrl
+	 * @param string $text
+	 */
+	public function addFooter($logoUrl, $text);


Also here I'd remove $logoUrl

Also let's make the text optional and by default go with the default footer :)

LukasReschke · 2017-04-07T09:09:37Z

Yikes… The Provisioning API has a "Resend welcome mail" endpoint which was implemented by copy-pasting from the user creation controller. Absolutely not error prone 😉 Let me adjust that…

LukasReschke · 2017-04-07T09:40:22Z

settings/Controller/UsersController.php

-				$mailData = array(
-					'username' => $username,
-					'url' => $link
+				$emailTemplate = new EMailTemplate($this->defaults);


Let's add a getEmailTemplate helper to IMailer instead :-)

Mhm. Actually not sure, separation of concerns et al, need to think about it :)

LukasReschke · 2017-04-07T11:40:40Z

There's a small bug in the theming code which I'm going to commit here as well, but then we already can have nicely themed mails:

… well, or at least: "somewhat nicely themed" 😉

LukasReschke · 2017-04-07T11:42:49Z

Note that I've removed the small gray Nextcloud logo at the bottom simply because that will cause a mess with theming and we should aim for consistency. And this is also already way nicer than before 😄

MorrisJobke · 2017-04-07T14:40:35Z

Review time

codecov-io · 2017-04-07T15:23:20Z

Codecov Report

Merging #4244 into master will increase coverage by 0.04%.
The diff coverage is 77.77%.

@@             Coverage Diff              @@
##             master    #4244      +/-   ##
============================================
+ Coverage     54.04%   54.08%   +0.04%     
- Complexity    21299    21329      +30     
============================================
  Files          1259     1259              
  Lines         74231    74303      +72     
============================================
+ Hits          40115    40186      +71     
- Misses        34116    34117       +1

Impacted Files	Coverage Δ	Complexity Δ
apps/provisioning_api/lib/AppInfo/Application.php	`100% <100%> (ø)`	`6 <0> (ø)`	⬇️
settings/Application.php	`100% <100%> (ø)`	`3 <0> (ø)`	⬇️
lib/private/Server.php	`93.13% <100%> (+0.14%)`	`120 <0> (ø)`	⬇️
...rovisioning_api/lib/Controller/UsersController.php	`83.96% <66.66%> (-0.22%)`	`107 <0> (ø)`
lib/private/Mail/EMailTemplate.php	`73.01% <73.01%> (ø)`	`21 <21> (?)`
settings/Mailer/NewUserMailHelper.php	`76.11% <76.11%> (ø)`	`8 <8> (?)`
settings/Controller/UsersController.php	`63.78% <83.33%> (-1.55%)`	`87 <0> (-2)`
apps/theming/lib/ThemingDefaults.php	`83.9% <86.66%> (+2.08%)`	`30 <1> (+3)`	⬆️
lib/public/AppFramework/Http/TemplateResponse.php	`70.83% <0%> (-25%)`	`9% <0%> (ø)`
lib/private/Security/CertificateManager.php	`92.78% <0%> (-1.04%)`	`38% <0%> (ø)`
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 325f925...8daf3d4. Read the comment docs.

MorrisJobke · 2017-04-07T16:58:07Z

I tested the changes and it works really nice 👍 from me

@Espina2

* thanks to @Espina2 for make this nice design * the button says "Set password" if the admin didn't specified a password Signed-off-by: Morris Jobke <[email protected]>

Signed-off-by: Lukas Reschke <[email protected]>

Add support for theming in generated emails and simplify API Signed-off-by: Lukas Reschke <[email protected]>

Signed-off-by: Lukas Reschke <[email protected]>

LukasReschke · 2017-04-07T17:06:53Z

👍

oparoz · 2017-04-07T17:18:42Z

What does the text version look like?

LukasReschke · 2017-04-07T17:24:20Z

What does the text version look like?

Thou shall take a look at the unit tests 😉

https://github.com/nextcloud/server/blob/8daf3d4a70bc97b6680956f6b13143e184a83c96/tests/data/emails/new-account-email-custom.txt

MorrisJobke · 2017-04-07T17:36:31Z

What does the text version look like?

Basically the same as before. It's only the text in the same order just without logo and styles.

Espina2 · 2017-04-07T18:43:50Z

I really prefer if after the Welcome a board, we have the name of the person, it would look like the email is written by one people and not autogenerated, looks more personal.

oparoz · 2017-04-07T19:25:09Z

Thank you guys :) 👍

MorrisJobke · 2017-04-07T19:45:55Z

I really prefer if after the Welcome a board, we have the name of the person, it would look like the email is written by one people and not autogenerated, looks more personal.

But in some cases we don't have the name. If the name is available we will put it there. 😉

MorrisJobke · 2017-04-07T19:46:18Z

if a display name is set for the user it says "Welcome aboard DISPLAYNAME"

cc @Espina2

MorrisJobke · 2017-04-07T19:52:25Z

Awesome sauce! 🚀

Espina2 · 2017-04-07T20:01:02Z

Awesome. Nice work Morris!

MorrisJobke · 2017-04-07T20:04:41Z

Awesome. Nice work Morris!

You had the major impact on this! You designed a really awesome template :) Thank you as well :) You rock 🚀 😄

jancborchardt · 2017-04-08T17:15:29Z

Great work everyone! :)

MorrisJobke added the 3. to review Waiting for reviews label Apr 6, 2017

MorrisJobke requested review from rullzer and jancborchardt April 6, 2017 18:23

MorrisJobke mentioned this pull request Apr 6, 2017

Better designed emails when sharing links and calendar etc #2071

Closed

MorrisJobke force-pushed the welcome-email branch from 97e1c1d to 8bbc1fb Compare April 7, 2017 05:23

MorrisJobke force-pushed the welcome-email branch from 8bbc1fb to 04ef6e5 Compare April 7, 2017 05:43

MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 7, 2017

LukasReschke approved these changes Apr 7, 2017

View reviewed changes

LukasReschke reviewed Apr 7, 2017

View reviewed changes

LukasReschke self-assigned this Apr 7, 2017

LukasReschke force-pushed the welcome-email branch from f955f16 to 64e2d88 Compare April 7, 2017 14:22

MorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 7, 2017

MorrisJobke requested review from nickvergessen and juliusknorr April 7, 2017 14:40

LukasReschke removed their assignment Apr 7, 2017

New layout for welcome email

0560e69

* thanks to @Espina2 for make this nice design * the button says "Set password" if the admin didn't specified a password Signed-off-by: Morris Jobke <[email protected]>

LukasReschke added 3 commits April 7, 2017 12:03

Fix tests

1be75e8

Signed-off-by: Lukas Reschke <[email protected]>

Add support for theming

281ad40

Add support for theming in generated emails and simplify API Signed-off-by: Lukas Reschke <[email protected]>

Update autoloader

8daf3d4

Signed-off-by: Lukas Reschke <[email protected]>

MorrisJobke force-pushed the welcome-email branch from 2108d60 to 8daf3d4 Compare April 7, 2017 17:04

MorrisJobke added design Design, UI, UX, etc. enhancement labels Apr 7, 2017

rullzer approved these changes Apr 7, 2017

View reviewed changes

MorrisJobke merged commit 8838ed4 into master Apr 7, 2017

MorrisJobke deleted the welcome-email branch April 7, 2017 19:52

MorrisJobke added this to the Nextcloud 12.0 milestone Apr 7, 2017

This was referenced Apr 11, 2017

New emails for sharebymail #4307

Merged

Update email template for lost password email #4308

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New layout for welcome email #4244

New layout for welcome email #4244

MorrisJobke commented Apr 6, 2017 •

edited

Loading

mention-bot commented Apr 6, 2017

MorrisJobke commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

Espina2 commented Apr 7, 2017

LukasReschke commented Apr 7, 2017 •

edited

Loading

LukasReschke left a comment

LukasReschke Apr 7, 2017

LukasReschke Apr 7, 2017

LukasReschke Apr 7, 2017

LukasReschke commented Apr 7, 2017 •

edited

Loading

LukasReschke Apr 7, 2017

LukasReschke Apr 7, 2017

LukasReschke commented Apr 7, 2017 •

edited

Loading

LukasReschke commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

codecov-io commented Apr 7, 2017 •

edited

Loading

MorrisJobke commented Apr 7, 2017

LukasReschke commented Apr 7, 2017

oparoz commented Apr 7, 2017

LukasReschke commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

Espina2 commented Apr 7, 2017

oparoz commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

Espina2 commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

jancborchardt commented Apr 8, 2017

New layout for welcome email #4244

New layout for welcome email #4244

Conversation

MorrisJobke commented Apr 6, 2017 • edited Loading

mention-bot commented Apr 6, 2017

MorrisJobke commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

Espina2 commented Apr 7, 2017

LukasReschke commented Apr 7, 2017 • edited Loading

LukasReschke left a comment

Choose a reason for hiding this comment

LukasReschke Apr 7, 2017

Choose a reason for hiding this comment

LukasReschke Apr 7, 2017

Choose a reason for hiding this comment

LukasReschke Apr 7, 2017

Choose a reason for hiding this comment

LukasReschke commented Apr 7, 2017 • edited Loading

LukasReschke Apr 7, 2017

Choose a reason for hiding this comment

LukasReschke Apr 7, 2017

Choose a reason for hiding this comment

LukasReschke commented Apr 7, 2017 • edited Loading

LukasReschke commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

codecov-io commented Apr 7, 2017 • edited Loading

Codecov Report

MorrisJobke commented Apr 7, 2017

LukasReschke commented Apr 7, 2017

oparoz commented Apr 7, 2017

LukasReschke commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

Espina2 commented Apr 7, 2017

oparoz commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

Espina2 commented Apr 7, 2017

MorrisJobke commented Apr 7, 2017

jancborchardt commented Apr 8, 2017

MorrisJobke commented Apr 6, 2017 •

edited

Loading

LukasReschke commented Apr 7, 2017 •

edited

Loading

LukasReschke commented Apr 7, 2017 •

edited

Loading

LukasReschke commented Apr 7, 2017 •

edited

Loading

codecov-io commented Apr 7, 2017 •

edited

Loading