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.

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

Need a way to get icon in primary text color #19565

Closed
nickvergessen opened this issue Feb 20, 2020 · 13 comments
Closed

Need a way to get icon in primary text color #19565

nickvergessen opened this issue Feb 20, 2020 · 13 comments
Labels
1. to develop Accepted and waiting to be taken care of feature: theming technical debt

Comments

@nickvergessen
Copy link
Member

nickvergessen commented Feb 20, 2020

As an app we want to have icons on primary color buttons.
For this we use $color-primary-text because we want it to be independent from the text color, because the text color in the button can be different from normal text color. So the following sounds like the solution:

	.icon-incoming-call {
		@include icon-color('video', 'actions', $color-primary-text, 1, true);
	}

However, $color-primary-text is white or black, and therefor

$varName: "--icon-#{$dir}-#{$icon}-#{$color}";
@if $core {
$varName: "--icon-#{$icon}-#{$color}";
}
#{$varName}: url(icon-color-path($icon, $dir, $color, $version, $core));

will use --icon-video-fff in case of default nextcloud blue, that is still okay.
But if you now enable dark-mode --icon-video-fff requests the image ?color=000 because:

/**
* Remove all matches from the $rule regex
*
* @param string $css string to parse
* @return string
*/
private function invertSvgIconsColor(string $css) {
return str_replace(
['color=000&', 'color=fff&', 'color=***&'],
['color=***&', 'color=000&', 'color=fff&'],
str_replace(
['color=000000&', 'color=ffffff&', 'color=******&'],
['color=******&', 'color=000000&', 'color=ffffff&'],
$css
)
);
}

Doesn't know anymore that fff is because of $color-primary-text 😿

Any clever idea around this? cc @skjnldsv @juliushaertl

@skjnldsv
Copy link
Member

Use svg icons inline or icons as font? and use color: var(--color-primary-text)
This is where we want to go in the end, icons classes are too messy.

@nickvergessen
Copy link
Member Author

This is where we want to go in the end, icons classes are too messy.

Sorry I'm just a backend dev in need of a icon in the color I tell you. So for now we get a wrong icon color in Talk dark mode, fine by me.

@nickvergessen
Copy link
Member Author

Onlyoffice has the same issue:
ONLYOFFICE/onlyoffice-nextcloud#247

@rullzer rullzer modified the milestones: Nextcloud 18.0.2, Nextcloud 18.0.3 Mar 11, 2020
@rullzer rullzer modified the milestones: Nextcloud 18.0.3, Nextcloud 18.0.4 Mar 23, 2020
@mat-m
Copy link

mat-m commented Apr 14, 2020

Mail has the same issue.
Inbox and Trash icons are black with Dark Theme from Accessibility.

@nickvergessen
Copy link
Member Author

nextcloud/spreed@6ddb58f seems to work as an intermediate workaround.

@blizzz
Copy link
Member

blizzz commented Nov 10, 2021

clearing the milestone

@juliusknorr
Copy link
Member

Is that actually needed now that more apps are switching to use vue material icons that embed SVG which can dynamically be adjusted in color? I'd say we can close this.

@nickvergessen
Copy link
Member Author

Still relevant for app icons with situations like the header bar or login page (e.g. registration app), but yeah less urgent

@skjnldsv
Copy link
Member

Do I get this right? You want an icon to have a fitting colour over a primary colour?
Like if it's too bright -> dark and if it's too dark -> bright ?

if so #31751 tackles that in a way

@nickvergessen
Copy link
Member Author

Yes exactly

@skjnldsv
Copy link
Member

#32060

@szaimen
Copy link
Contributor

szaimen commented Jan 9, 2023

Hi, please update to 24.0.8 or better 25.0.2 and report back if it fixes the issue. Thank you!

@szaimen szaimen added needs info 0. Needs triage Pending check for reproducibility or if it fits our roadmap and removed 1. to develop Accepted and waiting to be taken care of labels Jan 9, 2023
@nickvergessen nickvergessen added 1. to develop Accepted and waiting to be taken care of and removed bug needs info 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Jan 9, 2023
@nickvergessen
Copy link
Member Author

Still an issue atm, but we are all migrating away to inline SVGs, so it's hopefully solved soon.

@nickvergessen nickvergessen closed this as not planned Won't fix, can't repro, duplicate, stale Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of feature: theming technical debt
Projects
None yet
Development

No branches or pull requests

9 participants