-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Fix mail design #1446
Fix mail design #1446
Conversation
@eppfel, thanks for your PR! By analyzing the annotation information on this pull request, we identified @schiessle, @rullzer and @scolebrook to be potential reviewers |
I have found following project that aims for nice responsive emails: https://htmlemail.io/ maybe we lend some concepts from them ;) Edit: https://mjml.io/ also looks nice |
@eppfel what do you think of just using the blue logo there? That would look a lot nicer and simpler on white probably. Btw there’s a typo in the mail: »an Nextcloud account« |
@jancborchardt Yeah, blue logo would be more fitting, I guess. But that still leaves us with the theming question... |
I guess just changing for the correct logo is easy with the dynamic icon creation by @juliushaertl #840 ;) |
This should already be quite easy even without #840. Just for the theming stuff, it would of course be simpler to keep the blue backgorund with the white logo, just as it is used everywhere else in the Nextcloud UI at the moment. Otherwise we would again introduce a second logo, that may need some extra handling in case of theming. |
That was actually what I was referring to. |
Maybe then make it a blue circle, kinda like the stickers? That might look nicer than the rectangle which feels a bit misplaced. |
@eppfel what do you think about the circle idea? We could center that icon too maybe. And while we’re at improving the HTML mail, maybe also bold the username and the access link? |
@jancborchardt Sounds all good ideas. I'm just super busy ATM. |
|
I want to leave this as minor fix, instead of making it an overall Mail redesign. Because the state it is now, is not acceptable at all. We then even could look at backporting it. What do you think? @jancborchardt @MorrisJobke @schiessle |
6e3f6fd
to
07ac50d
Compare
I just rebased :) |
I have no idea why, but the "shared a file with you" email(left) has a wrong URL ('/themes/` should not be in there) but the "new user" email works fine(right): @eppfel: Any idea? And remind that I rebased and you need to fetch that version first. cc @rullzer @juliushaertl for this |
Ah found it - there is a new template ;) |
I just tested this and it works really nice 👍 |
Signed-off-by: Felix A. Epp <work@felixepp.de>
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
fc35a9c
to
224c89c
Compare
@MorrisJobke Signed, rebased ❗️, but typo and bold username/url are still there. I do not thoroughly understand transifex, yet. |
Current coverage is 56.99% (diff: 100%)@@ master #1446 diff @@
==========================================
Files 1191 1189 -2
Lines 72072 72037 -35
Methods 7299 7299
Messages 0 0
Branches 1210 1210
==========================================
- Hits 41057 41056 -1
+ Misses 31015 30981 -34
Partials 0 0
|
* a nextcloud * Strong username & url Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@eppfel I fixed the missing todo's. Please double check. Else good to go from my POV 👍 |
@@ -11,7 +11,7 @@ | |||
<td width="20px"> </td> | |||
<td style="font-weight:normal; font-size:0.8em; line-height:1.2em; font-family:verdana,'arial',sans;"> | |||
<?php | |||
print_unescaped($l->t('Hey there,<br><br>just letting you know that you now have an %s account.<br><br>Your username: %s<br>Access it: <a href="%s">%s</a><br><br>', array($theme->getName(), $_['username'], $_['url'], $_['url']))); | |||
print_unescaped($l->t('Hey there,<br><br>just letting you know that you now have a %s account.<br><br>Your username: <strong>%s</strong><br>Access it: <strong><a href="%s">%s</a></strong><br><br>', array($theme->getName(), $_['username'], $_['url'], $_['url']))); |
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.
Just out of interest? How does transifex allocate the translations, if the key changes?
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.
It shows a new untranslated string. but it does remember the original one and shows it in 'likely' matches or something. So that users have help in translating the new one.
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.
Ah, ok. So, it's fine changing these and not worry about the translation. Thanks for the clarification. ❤️
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.
Yep - just change it and it will show up after 3 am on transifex ;)
👍 |
As in issue #1326
I imitate the nextcloud header.
I replced the gif with the png, but maybe some other apps use it.
@nextcloud/designers
Todo
header, just with a circled logo