link from app-navigation-settings to personal settings#621
Conversation
szaimen
force-pushed
the
enh/noid/personal-settings-link
branch
19 times, most recently
from
ee7da82 to
834c3fc
Compare
css/style.scss
Outdated
|
|
||
| a { | ||
| display: block; | ||
| padding: 10px 0px 10px 27px; |
There was a problem hiding this comment.
Not a blocker for this PR but I see a lot of these magic numbers everywhere in Nextcloud codebase. Is there a plan to unify them a bit? Personally, I like the way this is done in bootstrap/tailwind with css utilities like p-1, p-2, p-3, p-4, p-5
There was a problem hiding this comment.
hm... cannot answer that. I just put them in place so that the link is good shaped for this specific situation...
There was a problem hiding this comment.
Is there a plan to unify them a bit? Personally, I like the way this is done in bootstrap/tailwind with css utilities like p-1, p-2, p-3, p-4, p-5
Would require further discussions with @nextcloud/server team :)
There was a problem hiding this comment.
did align it now to padding: 10px 0px 10px 25px which seems like the perfect values here...
There was a problem hiding this comment.
The general idea was more to move to shared vue components rather than implementing more global styles. For magic values we should slowly move more towards using css variables.
There was a problem hiding this comment.
Is it okay to merge this then or is there something I should change in this PR?
There was a problem hiding this comment.
@juliushaertl ok :) yeah shared Vue components make sense :)
@szaimen: Ship it 🚀
szaimen
force-pushed
the
enh/noid/personal-settings-link
branch
from
834c3fc to
7711080
Compare
szaimen
force-pushed
the
enh/noid/personal-settings-link
branch
from
7711080 to
ff78e91
Compare
Oh wow, how in the world did you manage to see that? 😱 It is fixed now :) |
templates/stream.app.navigation.php
Outdated
| </span> | ||
| <div id="activity-personal-settings-link"> | ||
| <a href="<?php p($_['personalSettingsLink']); ?>"> | ||
| <span class="no-icon"><?php p($l->t('Personal Activity Settings') . ' ↗'); ?></span> |
There was a problem hiding this comment.
I think we only use the ↗ for external links, but pinging @jancborchardt to clarify on that.
There was a problem hiding this comment.
We use either the icon-external or ↗ for external links. I think it's better not to use any icon here or maybe the activity app icon.
There was a problem hiding this comment.
Done. This is is how it looks without ↗:
Signed-off-by: szaimen <szaimen@e.mail.de>
szaimen
force-pushed
the
enh/noid/personal-settings-link
branch
from
ff78e91 to
f124ff6
Compare
|
/backport to stable22 |
|
/backport to stable21 |
|
/backport to stable20 |
6 participants
Fix #274
Signed-off-by: szaimen szaimen@e.mail.de