-
Notifications
You must be signed in to change notification settings - Fork 285
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
polish sidebar #215
polish sidebar #215
Conversation
Signed-off-by: Artem Anufrij <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #215 +/- ##
=======================================
Coverage 76.56% 76.56%
=======================================
Files 33 33
Lines 973 973
=======================================
Hits 745 745
Misses 228 228 |
Signed-off-by: Artem Anufrij <[email protected]>
Signed-off-by: Artem Anufrij <[email protected]>
Signed-off-by: Artem Anufrij <[email protected]>
Signed-off-by: Artem Anufrij <[email protected]>
Signed-off-by: Artem Anufrij <[email protected]>
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.
templates/part.card.php
Outdated
@@ -28,54 +28,45 @@ | |||
<?php p($l->t('by')); ?> | |||
<span>{{ cardservice.getCurrent().owner.displayname }}</span> | |||
</div> | |||
|
|||
<h3 style="padding-top: 0px; margin-top: 0px;"> |
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.
No inline styles please ;)
<?php p($l->t('Description')); ?> | ||
<a href="https://github.com/nextcloud/deck/wiki/Markdown-Help" target="_blank" class="icon-help" title="<?php p($l->t('Formatting help')); ?>"></a> | ||
<span class="save-indicator"><?php p($l->t('Saved')); ?></span> | ||
<div> |
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.
What is the need of two wrapping divs here? h3 is already a block element that could be used.
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.
for display: flex
.
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.
But we can just apply the flex styles to #card-meta h3
or am I missing something?
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.
display: flex
changes some margin/padding behavior. It looks some different in compare to standard h3
</div> | ||
|
||
|
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.
🙈 👍
@@ -926,6 +954,9 @@ button.button-inline:hover { | |||
margin-bottom: 0; | |||
} | |||
|
|||
.tabsContainer { | |||
margin-top: 15px; | |||
} |
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.
missing newline.
css/style.css
Outdated
} | ||
|
||
#card-meta .duedate .icon:hover { | ||
opacity: 0.7; |
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.
Not really needed, since the hover effect is done by the icon-delete class.
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.
There is just the delete button 😉
https://github.com/nextcloud/deck/pull/215/files#diff-a175750cce89c445e0a34d25823924c2R56
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.
but it doesn't change opacity. only color. for buttons we use always a different opacity on mouse over.
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 i see. Ok fine then.
css/style.css
Outdated
} | ||
|
||
#card-meta .duedate .icon { | ||
opacity: 0.3; |
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.
I would go to 0.5 opacity, since it should be default for all icons on nextcloud: nextcloud/server#5157
@juliushaertl |
Signed-off-by: Artem Anufrij <[email protected]>
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.
Fine by me now 👍
Signed-off-by: Artem Anufrij [email protected]