Skip to content

[stable27] fix(theming): stable 27 disable accessible color config switch#45422

Merged
skjnldsv merged 1 commit intostable27from
skjnldsv-patch-1
May 21, 2024
Merged

[stable27] fix(theming): stable 27 disable accessible color config switch#45422
skjnldsv merged 1 commit intostable27from
skjnldsv-patch-1

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented May 21, 2024 •
edited
Loading

Fix #39551

Only for 27! 28 and above have a different handling of themes and is fixed

Apply this fix

  1. add '27-disable-accessible-theming-color' => true, to your config.php
  2. delete the global folder on your data/appdata_xxxx/theming directory (no need to delete the theming folder)
  3. run an occ files:scan-app-data theming

How to reproduce the issue

  1. use a non accessible color like #004352
  2. load the files app
Before After
localhost_8081_index php_apps_files__dir=_ fileid=2 (1) localhost_8081_index php_apps_files__dir=_ fileid=2

@skjnldsv
fix(theming): stable 27 disable accessible color config switch
dc40dcb 
Signed-off-by: John Molakvoæ <skjnldsv@users.noreply.github.com>
@skjnldsv skjnldsv added bug design Design, UI, UX, etc. 3. to review Waiting for reviews feature: theming labels May 21, 2024
@skjnldsv skjnldsv added this to the Nextcloud 27.1.10 milestone May 21, 2024
@skjnldsv skjnldsv requested review from a team, AndyScherzinger and artonge May 21, 2024 14:25
@skjnldsv skjnldsv self-assigned this May 21, 2024
@skjnldsv skjnldsv requested review from nfebe and szaimen and removed request for a team May 21, 2024 14:25
@szaimen
Copy link
Contributor

szaimen commented May 21, 2024 •
edited
Loading

28 and above have a different handling of themes and is fixed

I fear it is not, seems to behave completely the same, just tested (it still adjusts the color to an accessible one)...

primary color (black):
image
folder color (grey):
image

@artonge
Copy link
Contributor

artonge commented May 21, 2024

folder color (grey):

But that is expected, no? Black on black does not work for accessibility.

@szaimen
Copy link
Contributor

szaimen commented May 21, 2024 •
edited
Loading

folder color (grey):

But that is expected, no? Black on black does not work for accessibility.

yes, this is also what I am understanding but seems like the customer complains about exactly this, no?

@AndyScherzinger
Copy link
Member

AndyScherzinger commented May 21, 2024

yes, this is also what I am understanding but seems like the customer complains about exactly this, no?

No the complaint is the before image which shows the folder color used for dark and light

@skjnldsv
Copy link
Member Author

skjnldsv commented May 21, 2024 •
edited
Loading

yes, this is also what I am understanding but seems like the customer complains about exactly this, no?

No the complaint is the before image which shows the folder color used for dark and light

28 with the color above:
image

EDIT: @AndyScherzinger the colour shift is till being done, but the computation is different to be accessible.
We might have to check with the customer again that they're ok with it

@szaimen
Copy link
Contributor

szaimen commented May 21, 2024

yes, this is also what I am understanding but seems like the customer complains about exactly this, no?

No the complaint is the before image which shows the folder color used for dark and light

I see. 👍

@AndyScherzinger
Copy link
Member

AndyScherzinger commented May 21, 2024

We might have to check with the customer again that they're ok with it

Yes, we might. The main issue is with the light theme though. The light folder icons in your before screenshot are also used in light mode on 27...

So the main issue to be addressed with this fix is:
2024-05-21 16_43_47-Files - Cloud – Mozilla Firefox

@AndyScherzinger
Copy link
Member

AndyScherzinger commented May 21, 2024 •
edited
Loading

@skjnldsv how would the light theme look like in 28 with that color? That is the main concern with the given color code

@skjnldsv
Copy link
Member Author

skjnldsv commented May 21, 2024

@skjnldsv how would the light theme look like in 28 with that color? That is the main concern with the given color code

image
Because we fixed the computation on the client side, so we now which theme is active 👍

@AndyScherzinger
Copy link
Member

AndyScherzinger commented May 21, 2024 •
edited
Loading

Sweet, so v28 looks good to me 👍
Can't tell if this works fine with they background image as already mentioned by @artonge to me.

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels May 21, 2024
@skjnldsv skjnldsv merged commit 51adff2 into stable27 May 21, 2024
@skjnldsv skjnldsv deleted the skjnldsv-patch-1 branch May 21, 2024 15:30
@skjnldsv skjnldsv mentioned this pull request May 22, 2024
Merged
10 tasks
@susnux susnux moved this from 🏗️ In progress to ☑️ Done in 📁 Files team Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

@artonge artonge artonge approved these changes

@nfebe nfebe Awaiting requested review from nfebe nfebe was automatically assigned from nextcloud/server-frontend

@szaimen szaimen Awaiting requested review from szaimen szaimen was automatically assigned from nextcloud/server-frontend

Assignees

@skjnldsv skjnldsv

Labels

4. to release Ready to be released and/or waiting for tests to finish bug design Design, UI, UX, etc. feature: theming

Projects

📁 Files team
Status: ☑️ Done

Milestone

Nextcloud 27.1.10

Development

Successfully merging this pull request may close these issues.

4 participants

@skjnldsv @szaimen @artonge @AndyScherzinger