Invert logo colour based on current theme #9

Manually merged
dalahast merged 1 commits from dynamic-logo-color into master 2021-01-22 16:37:19 +01:00
Owner

impacts visibility.
on light navbar inverts the logo colour (originally white) adding inline style="filter: invert(.8);".

screenshot light:
image

screenshot dark:
image

impacts visibility. on light navbar inverts the logo colour (originally white) adding inline `style="filter: invert(.8);"`. screenshot light: ![image](/attachments/7d4bce8a-8245-4c86-82d5-3ee9f96ae5a5) screenshot dark: ![image](/attachments/85d12e5f-0e68-43dd-922d-a7920f844445)
wanderer self-assigned this 2021-01-22 14:03:11 +01:00
dalahast was assigned by wanderer 2021-01-22 14:03:11 +01:00
wanderer added 1 commit 2021-01-22 14:03:12 +01:00
wanderer added this to the UX project 2021-01-22 14:05:06 +01:00
wanderer requested review from dalahast 2021-01-22 14:05:10 +01:00
Author
Owner

@dalahast please review an merge if ok.

@dalahast please review an merge if ok.
dalahast reviewed 2021-01-22 15:09:23 +01:00
@ -2,3 +2,3 @@
<div class="item brand" style="justify-content: space-between;">
<a href="/">
<img class="ui mini image" src="/img/logo.svg" alt="git.dotya.ml icon">
<img class="ui mini image" {{if .SignedUser}}{{if not (eq .SignedUser.Theme "arc-green" "42l-dark" "42l-light")}}style="filter: invert(.8);"{{end}}{{else if not (eq DefaultTheme "arc-green" "42l-dark" "42l-light")}}style="filter: invert(.8);"{{end}} src="/img/logo.svg" alt="git.dotya.ml icon">
Owner

Isn't there a dark/light parameter that could be used instead of an excplicit theme name? This is by all means ok for the current theme selection, but it would be better if this could be "set and forget".

Isn't there a dark/light parameter that could be used instead of an excplicit theme name? This is by all means ok for the current theme selection, but it would be better if this could be "set and forget".
Author
Owner

this would of course be better served as part of each theme but that would require maintaining a fork for each theme whereas this is not too elegant but still easier - at least as a hotfix.

this would of course be better served as part of each theme but that would require maintaining a fork for each theme whereas this is not too elegant but still easier - at least as a hotfix.
Owner

Right, the themes are not expected to change for the foreseeable future so this is perfectly fine and indeed certainly better than maintaining a fork of each theme.

Right, the themes are not expected to change for the foreseeable future so this is perfectly fine and indeed certainly better than maintaining a fork of each theme.
Author
Owner

it is kind of hacky but for now it's still better than the other solution :D or at least quicker.
it might take its toll on the server, since it needs to evaluate DefaultTheme and .SignedUser.Theme for each request, but that would be a bigger problem under different loads..
it's good to know what potential bottlenecks are, though.

it is kind of hacky but for now it's still better than the other solution :D or at least quicker. it might take its toll on the server, since it needs to evaluate `DefaultTheme` and `.SignedUser.Theme` for each request, but that would be a bigger problem under different loads.. it's *good* to know what potential bottlenecks are, though.
dalahast marked this conversation as resolved
dalahast approved these changes 2021-01-22 16:37:06 +01:00
dalahast manually merged commit 8ad2672dbe into master 2021-01-22 16:37:19 +01:00
dalahast deleted branch dynamic-logo-color 2021-01-22 16:37:49 +01:00
Sign in to join this conversation.
No description provided.