Make unread counter primary colour

[GretaD]

tested manually on mail, i cannot connect to nc/vue

fixes #3054

[juliusknorr]

I'd suggest that we use the primary-element colors so that if the primary color is a light grey for example, we fallback to a value that still has enough contrast.

[raimund-schluessler]

I'd suggest that we use the primary-element colors so that if the primary color is a light grey for example, we fallback to a value that still has enough contrast.

At least in the docs, this is hardly even readable. But maybe the variables in the docs need an update, I didn't test with server though.

[raimund-schluessler]

Is this how it's supposed to look like?

I think the contrast for the Normal Counter is terrible. @jancborchardt @nimishavijay Can you have a look, please?

[GretaD]

Is this how it's supposed to look like?

no, it is suppose to look like my screenshot on the description

[GretaD]

yeah, maybe we need --color-primary-element-lighter, instead of light @juliushaertl ?

[juliusknorr]

Either that or rather adjust it so that it matches the calculation of --color-primary-light depending on how --color-primary-element-light is currently used. I'll try to check the usages on our existing code base

[juliusknorr]

See nextcloud/server#33641

[juliusknorr]

Tested and works in combination with the server pr

[raimund-schluessler]

Tested and works in combination with the server pr

If this requires a server PR, it means it will only work with fixed servers. Since this affects a component already widely used, this will be quite problematic. It could mean that this component in nextcloud/vue 6.0.0 will only work with server 25 and above (unless nextcloud/server#33641 is backported and widely distributed).

[ChristophWurst]

It could mean that this component in nextcloud/vue 6.0.0 will only work with server 25 and above (unless nextcloud/server#33641 is backported and widely distributed).

Yes, same issue as #3057 (comment). The plan is to have another major release v7 that will be Nextcloud 25+ only.

[raimund-schluessler]

It could mean that this component in nextcloud/vue 6.0.0 will only work with server 25 and above (unless nextcloud/server#33641 is backported and widely distributed).

Yes, same issue as #3057 (comment). The plan is to have another major release v7 that will be Nextcloud 25+ only.

Then let's not merge this until we have 6.0.0 stable release. Otherwise we need to backport every PR/fix to a stable6 branch already.

[skjnldsv]

If this requires a server PR, it means it will only work with fixed servers. Since this affects a component already widely used, this will be quite problematic

We can add variables fallback. If they are not defined, they will use the second value

[raimund-schluessler]

If this requires a server PR, it means it will only work with fixed servers. Since this affects a component already widely used, this will be quite problematic

We can add variables fallback. If they are not defined, they will use the second value

I would prefer that over a server 25+ only nc/vue release.

[raimund-schluessler]

The fallback works, but could we maybe adjust the value of --color-primary-element-light to the one of server 25+ so it doesn't look weird in the docs.

[raimund-schluessler]

Sorry to bother everyone again, but using these adjustments on a server running NC 24.0.4 this is the result:

• with primary color 575757:
  
• and with default one 0082C9:
  

So, I think this fallback still does not work, because --color-primary-element-light seems to exist for NC 24 already.

[GretaD]

Sorry to bother everyone again, but using these adjustments on a server running NC 24.0.4 this is the result:

• with primary color 575757:
  
• and with default one 0082C9:
  

So, I think this fallback still does not work, because --color-primary-element-light seems to exist for NC 24 already.

no sorry, thank you for checking it so carefully.
@juliushaertl should we rename --color-primary-element-light to --color-nc-primary-element-light on your pr here: https://github.com/nextcloud/server/pull/33641/files ? i can do the change, just to confirm

[skjnldsv]

I think this fallback still does not work

Yep, it only works if you add a new css variable, which I thought y'all were doing for 25 :)

[GretaD]

Yep, it only works if you add a new css variable, which I thought y'all were doing for 25 :)

so what do you propose instead of --color-nc-primary-element-light?

[juliusknorr]

For reference in the comments as well, we can use -lighter here as the default and fallback to -light, as lighter is no longer present on the new nextcloud release then and the old -lighter color is close enough to the new one.

delivered

[ChristophWurst]

My only remark is that color is spelled without a u

[ChristophWurst]

With the help of the fallback this is no longer a breaking change, right?

[juliusknorr]

Yes.

[nickvergessen]

This totally breaks as shown on the screenshots above.

In talk the unread counter for mentioned and unmentioned almost look the same apart from the broken font color that doesn't have enough contrast

If you need a "primary" unread counter, you need to specify the type="highlighted" on NcCounterBubble
If you use it via NcListItem like Talk, the primary unread counter is available counterType="highlighted"

Can we revert this?

[ChristophWurst]

Only an issue with dark theme and highlighted counters -> #3249