Bugfix/2 fa settings icons invert in dark themes #13643

[Maders]

Other two factor auth apps must implement your own icon-twofactor_$appId css class based on this mixin functionality.
E.g. totp app:
.icon-twofactor_totp { @include icon-black-white('icon-name', 'app-name', 1) }
// result is something like that

But i can't understand how nextcloud theme system, know to change the icons color with svg api based on current theme mode.
Sorry, i know my English grammar is not correct 😬 but i like to contribute.

[rullzer]

@juliushaertl @skjnldsv

[juliusknorr]

@rullzer This is something where the some inline-css approach / backend icon registration would make sense, since otherwise every provider would need to load their own css file. Also if we want to move other backend-provided icons from urls to classes (like for navigation/activity)

[juliusknorr]

Thanks @Maders The only issue I see with this approach would be as mentioned in the first post that every twofactor provider will then need to load an individual CSS file. Probably ok for twofactor, since there will most likely just be a hand full of different providers. What do you think @skjnldsv @ChristophWurst

[ChristophWurst]

Other two factor auth apps must implement your own icon-twofactor_$appId css class based on this mixin functionality.

I guess this is still possible with webpack bundling, right @juliushaertl?

[skjnldsv]

@juliushaertl I'm ok with that, but @rullzer might not since he's trying to remove as many style/script request on load as he can 😝

[rullzer]

Well yes I want as less css loaded as possible ;) (We still load to much).

Anyways. What do we do here. Merge for 16 or 17? @skjnldsv @ChristophWurst @juliushaertl ?

[skjnldsv]

@juliushaertl and I planned to sit and discuss again the icon registration system at the Contributor week.
I'd say, let's wait until then so we know where we'll head?

[MorrisJobke]

@juliushaertl and I planned to sit and discuss again the icon registration system at the Contributor week.
I'd say, let's wait until then so we know where we'll head?

@skjnldsv @juliushaertl @ChristophWurst 🏓

[skjnldsv]

We still don't know where we'll head 🙈

[skjnldsv]

@juliushaertl we tlked about this last week, shall we take a position here? :)

[juliusknorr]

When thinking about that again, the icon font approach you proposed doesn't work if we use it as a background image, or am I missing something?

[skjnldsv]

When thinking about that again, the icon font approach you proposed doesn't work if we use it as a background image, or am I missing something?

Nope, it only work as a pseudo element, that means we can use color: var(--xxx).
But it also takes space in the dom.
Let's move to a more simpler option. Like you suggested, add a class on body like theme-light/theme-dark and start using it?

We can then easily implement this in the scss svg mixin as well :)

[juliusknorr]

Let's move to a more simpler option. Like you suggested, add a class on body like theme-light/theme-dark and start using it?
We can then easily implement this in the scss svg mixin as well :)

👍