Make user iconBackgrounds more flexible

[djensen47]

According to this line of code:

NodeBB/src/user/data.js

Line 21 in e17c4b2

var iconBackgrounds = [

The background colors used for a the user icon background is hard-coded in an array. This can't be overridden even with styles. Instead of using inline styles what about using a class instead and then having default values set for that class?

	var iconBackgrounds = [
		'icon-bg-01', 'icon-bg-02', 'icon-bg-03', 'icon-bg-04', 'icon-bg-05', 'icon-bg-06',
	];

Ans instead of setting background-color add this class instead.

Thoughts?

[barisusakli]

I have no objections to making the colors customizable 👍

[djensen47]

👍 Now that I have 2 changes, I'll submit a PR this week for this. 😁

I just need to get a dev environment setup locally.

[pitaj]

I recommend you don't make this a breaking change. That is, still provide the values for the inline style version so you don't need to fix every theme.

[djensen47]

I recommend you don't make this a breaking change. That is, still provide the values for the inline style version so you don't need to fix every theme.

I don't think that will work. The problem is that inline styles always override classes.

What do you normally do for changes like this? Does it need to become a config or admin setting now?

[djensen47]

Correction: it looks like !important will actually override inline styles. (I should've known this)

I try not to use !important because it's usually feels like an anti-pattern, especially in this case. But if that's how we need to do it to be backward compatible, then that's the solution.

[pitaj]

I try not to use !important because it's usually feels like an anti-pattern

You wouldn't need it. All you'd need to do is not include the inline style portion in themes which implement the class portion.

For instance, in Persona, you could replace this line:

<div class="media-object avatar avatar-lg" style="background-color: {target.user.icon:bgColor}">{target.user.icon:text}</div>

With this:

<div class="media-object avatar avatar-lg {target.user.icon:bgColorClass}">{target.user.icon:text}</div>

Then there is no conflict, and you don't need !important

Does it need to become a config or admin setting now?

If you're asking about how the colors should be configurable, I'd say that the class definitions should be up to the theme.

[djensen47]

Oh, cool. That's how I was thinking of doing it in the first place. 😁

I was thinking of child themes or other themes that might need to rely on those values. To solve that, I'm thinking we just keep inline style values available and make them deprecated?

[julianlam]

While we're here, can someone take a look at feasibility of doing the text label with a pseudo-element instead? I have a sneaking suspicion that it might be:

• Easier to center vertically
• Play nicer with images

If we don't involve a text node inside... right now I think the text label avatars are a pixel higher than equivalent images, even though they are the same size in CSS.