Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixes visual bug by making the icon grey only on hover and white otherwise. #25023

Closed
wants to merge 2 commits into from
Closed

Fixes visual bug by making the icon grey only on hover and white otherwise. #25023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fef42f4
Fixes #24450 by making the icon grey only on hover and white otherwise.
RafaOP Jan 8, 2021
2081939
Fixes #24450 by making the icon grey only on hover and white otherwise.
RafaOP May 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion apps/settings/css/settings.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,9 @@ select {
border-color: var(--color-primary) !important;
}

&.icon-file {
&.icon-file,
&.icon-file-white
{
padding-left: 48px;
background-position: 24px;
}
Expand Down
2 changes: 1 addition & 1 deletion apps/settings/templates/settings/personal/development.notice.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<div class="section development-notice">
<p>
<a href="<?php p($_['reasons-use-nextcloud-pdf-link']); ?>" id="open-reasons-use-nextcloud-pdf" class="link-button icon-file" target="_blank"><?php p($l->t('Reasons to use Nextcloud in your organization'));?></a>
<a href="<?php p($_['reasons-use-nextcloud-pdf-link']); ?>" id="open-reasons-use-nextcloud-pdf" class="link-button icon-file-white" target="_blank"><?php p($l->t('Reasons to use Nextcloud in your organization'));?></a>
</p>
<p>
<?php print_unescaped(str_replace(
Expand Down
2 changes: 1 addition & 1 deletion apps/theming/css/theming.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ $invert: luma($color-primary) > 0.6;
}

/* Colorized svg images */
.icon-file, .icon-filetype-text {
.icon-file, .icon-filetype-text, .icon-file-white:hover, .icon-file-white:focus {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juliushaertl please confirm.
If so, it's good for me

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem to be the right place for just the focus styling. This should only apply for the button so it should rather be part of settings.scss if even needed as the button should have a focus/hover feedback already.

background-image: url(./img/core/filetypes/text.svg?v=#{$theming-cachebuster});
}

Expand Down
5 changes: 5 additions & 0 deletions core/css/icons.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,11 @@ audio, canvas, embed, iframe, img, input, object, video {
@include icon-color('text', 'filetypes', #969696, 1, true);
}

.icon-file-white,
.icon-filetype-text-white {
@include icon-color('text', 'filetypes', $color-white, 1, true);
}

Comment on lines +391 to +395
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-white is not ideal as the button will have a primary color adapted to the theming, so whenever there is a bright theme used the white fixed does not fit anymore. I'd suggest something like this:

Suggested change
.icon-file-white,
.icon-filetype-text-white {
@include icon-color('text', 'filetypes', $color-white, 1, true);
}
.icon-file-primary,
.icon-filetype-primary {
@include icon-color('text', 'filetypes', $color-primary-text, 1, true);
}

.icon-filetype-file {
@include icon-color('file', 'filetypes', #969696, 1, true);
}
Expand Down