Conversation
|
@karlitschek, thanks for your PR! By analyzing the annotation information on this pull request, we identified @rullzer, @nickvergessen and @MorrisJobke to be potential reviewers |
| image_path('core', 'twitter.svg'), | ||
| image_path('core', 'rss.svg'), | ||
| image_path('core', 'mail.svg'), | ||
| '<a target="_blank" href="https://plus.google.com/b/104036748063781940910/104036748063781940910/about">', |
There was a problem hiding this comment.
Please add rel="noreferrer noopener" to all external links, otherwise the opening page can control the window object. See https://mathiasbynens.github.io/rel-noopener/ for more details.
So basically somebody clicks the link to Twitter and Twitter can make the tab where the link has been clicked change to any other URL without you noticing.
|
Works 👍 Some small nitpicks in the comments above :-) |
|
looks nice 👍 |
LukasReschke
force-pushed
the
socialbuttons
branch
2 times, most recently
from
d23215b to
0047059
Compare
|
svg's should be optimized for size (forgot the name of the tool @jancborchardt used) |
| ], | ||
| $l->t(' | ||
| <p class="social-button"> | ||
| {googleopen}<img width="50" src="{googleimage}" title="Follow us on Google Plus!" alt="Follow us on Google Plus!">{linkclose} |
There was a problem hiding this comment.
👎 Any specific reason why HTML was put into the translation string? I guess we should only have text in the translated strings, e.g. Follow us on Google Plus! and then decorate the translated string with the HTML around, e.g. the image tag. Otherwise translators might be confused about this syntax and even mess it up for some languages.
There was a problem hiding this comment.
example:
'<p class="social-button">
{googleopen}<img width="50" src="{googleimage}" title="' . $l->t('Follow us on Google Plus!') . '" alt="' . $l->t('Follow us on Google Plus!') . '">{linkclose}'
svgo |
MorrisJobke
added
2. developing
|
No, the tool is called scour. (svgo is too lossy). The line from core/img/image-optimization.sh can be used. |
|
Compressed the icons and fixed some CSS. @karlitschek what about @ChristophWurst’s comments above? |
|
@jancborchardt @ChristophWurst Hehe. Yes. Totally true. I just had no time to fix this yet. Feel free... |
|
Fixed the translation of HTML. |
rullzer
added
3. to review
|
👍 |
1 similar comment
|
👍 |
|
...we don't want this in Nextcloud 10, right?! @karlitschek |
|
Awesome guys. Thanks for finishing this! I think this is not critical but the risk for 10 is also low. So feel free to backport if wanted :-) |
|
I would drop the backport of this. Nothing crucial in my opinion. |
|
Agreed |
adding social buttons
11 participants
Adding social buttons to the personal and admin page as planed here: #745