Change color of error and success #39391

JuliaKirschenheuter · 2023-07-14T11:03:33Z

Resolves: [BITV] Special buttons color contrast improvements nextcloud-libraries/nextcloud-vue#4324

Summary

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)

Pytal · 2023-07-17T22:59:28Z

Viable to use the --color-[error|warning|success|info]-text variables added for #38211 even if it's used as a background in this context?

Otherwise if we're making the --color-* variables meet the 4.5:1 contrast ratio then let's drop the --color-*-text variables

Pytal · 2023-07-17T23:03:21Z

^ which means we need to switch https://github.com/nextcloud/nextcloud-vue/blob/208bb0903785c9d419ef43b70d696c83a5e8af90/src/components/NcInputField/NcInputField.vue#L456-L462 back to the non text variables

JuliaKirschenheuter · 2023-07-19T11:24:12Z

Thank you @Pytal,
do you mean i should replace https://github.com/nextcloud/nextcloud-vue/blob/208bb0903785c9d419ef43b70d696c83a5e8af90/src/components/NcInputField/NcInputField.vue#L456-L462 with --color-error and --color-warning and replace codes of them to the new ones from Nimisha?

Pytal · 2023-07-20T00:35:18Z

Yes, if we go with these new colors then in https://github.com/nextcloud/nextcloud-vue/blob/208bb0903785c9d419ef43b70d696c83a5e8af90/src/components/NcInputField/NcInputField.vue#L456-L462 we should use --color-error and --color-success instead @JuliaKirschenheuter

Which would also mean that we need to make sure the other colors in

server/apps/theming/css/default.css

Lines 20 to 35 in 3da48c2

    
           --color-error: #e9322d; 
        
           --color-error-rgb: 233,50,45; 
        
           --color-error-hover: #ed5a56; 
        
           --color-error-text: #e7201b; 
        
           --color-warning: #c28900; 
        
           --color-warning-rgb: 194,137,0; 
        
           --color-warning-hover: #cea032; 
        
           --color-warning-text: #996c00; 
        
           --color-success: #3fa857; 
        
           --color-success-rgb: 63,168,87; 
        
           --color-success-hover: #65b978; 
        
           --color-success-text: #318344; 
        
           --color-info: #006aa3; 
        
           --color-info-rgb: 0,106,163; 
        
           --color-info-hover: #3287b5; 
        
           --color-info-text: #006aa3;

also meet the minimum 4.5:1 contrast ratio before we can remove the --color-*-text variables

JuliaKirschenheuter · 2023-07-21T13:34:10Z

we should use --color-error and --color-success instead

Thank you @Pytal!

Which would also mean that we need to make sure the other colors in server/apps/theming/css/default.css also meet the minimum 4.5:1 contrast ratio before we can remove the --color-*-text variables

@nimishavijay could you please make a check of a contrast for all this variables please? 🙏

JuliaKirschenheuter · 2023-07-24T13:52:16Z

waiting for Nimisha's feedback

JuliaKirschenheuter · 2023-07-24T13:52:19Z

I will finish this PR after my vacation

nimishavijay

Checking the contrast for these colors, it's seen that most of these don't meet the requirements.

❌ --color-error: #e9322d; 
❌ --color-error-rgb: 233,50,45;  
❌ --color-error-hover: #ed5a56; 
✅ --color-error-text: #e7201b; 

❌ --color-warning: #c28900; 
❌ --color-warning-rgb: 194,137,0; 
❌ --color-warning-hover: #cea032; 
✅ --color-warning-text: #996c00;  

❌ --color-success: #3fa857; 
❌ --color-success-rgb: 63,168,87; 
❌ --color-success-hover: #65b978; 
✅ --color-success-text: #318344; 

✅ --color-info: #006aa3; 
✅ --color-info-rgb: 0,106,163; 
❌ --color-info-hover: #3287b5;  
✅ --color-info-text: #006aa3;

Here are the recommended colors for the variables. Do note that the previous suggestion in this PR is not suitable anymore, because for the hover version of a color we would lighten it slightly, and this would not meet contrast requirements. So I have revised the colors such that the hover color will also meet contrast requirements. All of the following colors have a contrast ratio of at least 4.5:1 against #ffffff

--color-error: #CB1710; 
--color-error-rgb: 203,23,16;  
--color-error-hover: #E71A13; 
--color-error-text: #CB1710; 

/* All elements with warning color will be replaced by error or a secondary button, so we don't need to change this */
--color-warning: #c28900; 
--color-warning-rgb: 194,137,0; 
--color-warning-hover: #cea032; 
--color-warning-text: #996c00;  

--color-success: #2C773C; 
--color-success-rgb: 44,119,60; 
--color-success-hover: #328644; 
--color-success-text: #2C773C; 

--color-info: #006aa3; 
--color-info-rgb: 0,106,163; 
--color-info-hover: #1f7cae;  
--color-info-text: #006aa3;

Would this work? :)

JuliaKirschenheuter · 2023-08-11T07:42:07Z

Dear @nimishavijay,

Thank you for your suggestions!

Unfortunately in most cases i would need a mix for color and not a code ;( Could you please walk through them like in #39317 (comment)? See

server/apps/theming/lib/Themes/DefaultTheme.php

Lines 147 to 149 in c133119

    
           '--color-error-rgb' => join(',', $this->util->hexToRGB($colorError)), 
        
           '--color-error-hover' => $this->util->mix($colorError, $colorMainBackground, 60), 
        
           '--color-error-text' => $this->util->darken($colorError, 4),

Could you please test them in dark mode too (i know it is lots of work but we can't walk around it)?

nimishavijay · 2023-08-16T17:24:03Z

How about the following colors? All the colors have a contrast of at least 3:1 against their appropriate background colors, and the text colors have a contrast of 4.5:1. This should cover all cases.

// LIGHT MODE

--color-error: #D91812
--color-error-hover: #dd342f
--color-error-rgb: (217,24,18)
--color-error-text: #c61610


--color-success: #2D7B41
--color-success-hover: #448955
--color-success-rgb: (45,123,65)
--color-success-text: #286c39

--color-info: #0071AD
--color-info-hover: #197fb5
--color-info-rgb: (45,123,65)
--color-info-text: #006499

// DARK MODE

--color-error: #D91812
--color-error-hover: #ca1712
--color-error-rgb: (217,24,18)
--color-error-text: #ef3f3a


--color-success: #2D7B41
--color-success-hover: #2b733d
--color-success-rgb: (45,123,65)
--color-success-text: #35914d

--color-info: #0071AD
--color-info-hover: #016aa1
--color-info-rgb: (0,113,173)
--color-info-tex: #008fdb

Mix for obtaining the colors in light mode


// DefaultTheme.php

$colorError = '#D91812';
$colorSuccess = '#2D7B41';
$colorInfo = '#0071AD';

'--color-error' => $colorError,
'--color-error-rgb' => join(',', $this->util->hexToRGB($colorError)),
'--color-error-hover' => $this->util->mix($colorError, $colorMainBackground, 75),
'--color-error-text' => $this->util->darken($colorError, 4),

'--color-success' => $colorSuccess,
'--color-success-rgb' => join(',', $this->util->hexToRGB($colorSuccess)),
'--color-success-hover' => $this->util->mix($colorSuccess, $colorMainBackground, 78),
'--color-success-text' => $this->util->darken($colorSuccess, 4),

'--color-info' => $colorInfo,
'--color-info-rgb' => join(',', $this->util->hexToRGB($colorInfo)),
'--color-info-hover' => $this->util->mix($colorInfo, $colorMainBackground, 80),
'--color-info-text' => $colorInfo,

In dark mode

// DarkTheme.php

$colorError = '#D91812';
$colorSuccess = '#2D7B41';
$colorInfo = '#0071AD';

'--color-error' => $colorError,
'--color-error-rgb' => join(',', $this->util->hexToRGB($colorError)),
'--color-error-hover' => $this->util->mix($colorError, $colorMainBackground, 85),
'--color-error-text' => $this->util->lighten($colorError, 12),

'--color-success' => $colorSuccess,
'--color-success-rgb' => join(',', $this->util->hexToRGB($colorSuccess)),
'--color-success-hover' => $this->util->mix($colorSuccess, $colorMainBackground, 85),
'--color-success-text' => $this->util->lighten($colorSuccess, 6),

'--color-info' => $colorInfo,
'--color-info-rgb' => join(',', $this->util->hexToRGB($colorInfo)),
'--color-info-hover' => $this->util->mix($colorInfo, $colorMainBackground, 85),
'--color-info-text' => $this->util->lighten($colorInfo, 9),

Also ccing @nextcloud/designers here because the color for --color-info has been changed to also be accessible in dark mode. Now there are 4 shades of blue in the UI, --color-primary, --color-primary-element, --color-primary-element-light and --color-info as well as all their hover variants. We can think about if we want to consolidate these into just 2 or 3 colors :)

skjnldsv · 2023-08-18T18:13:44Z

There was 1 failure:
295 |  
296 | 1) OCA\Theming\Tests\Service\DefaultThemeTest::testThemindDisabledFallbackCss
297 | Failed asserting that two strings are equal.
298 | --- Expected
299 | +++ Actual
300 | @@ @@
301 | --color-success-hover: #448955;\n
302 | --color-success-text: #286c39;\n
303 | --color-info: #0071ad;\n
304 | -  --color-info-rgb: 0,113,173;\n
305 | +  --color-info-rgb: 45,123,65;\n
306 | --color-info-hover: #197fb5;\n
307 | -  --color-info-text: #0071ad;\n
308 | +  --color-info-text: #006499;\n
309 | --color-loading-light: #cccccc;\n
310 | --color-loading-dark: #444444;\n
311 | --color-box-shadow-rgb: 77,77,77;\n
312 |  
313 | /drone/src/apps/theming/tests/Themes/DefaultThemeTest.php:161

Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>

JuliaKirschenheuter added the 3. to review Waiting for reviews label Jul 14, 2023

JuliaKirschenheuter requested review from skjnldsv, Pytal and nimishavijay July 14, 2023 11:03

JuliaKirschenheuter self-assigned this Jul 14, 2023

JuliaKirschenheuter requested a review from artonge July 14, 2023 11:28

artonge approved these changes Jul 17, 2023

View reviewed changes

nimishavijay approved these changes Jul 18, 2023

View reviewed changes

AndyScherzinger approved these changes Jul 20, 2023

View reviewed changes

nimishavijay self-assigned this Jul 24, 2023

nimishavijay requested changes Jul 24, 2023

View reviewed changes

skjnldsv removed their request for review August 1, 2023 09:13

JuliaKirschenheuter force-pushed the fix/4324-fix-color-of-special-buttons branch 2 times, most recently from 0ff701a to 811ecc5 Compare August 18, 2023 16:14

JuliaKirschenheuter requested a review from nimishavijay August 18, 2023 16:15

Pytal approved these changes Aug 18, 2023

View reviewed changes

nimishavijay approved these changes Aug 18, 2023

View reviewed changes

skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Aug 18, 2023

Change color of error and success

d6fb961

Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>

JuliaKirschenheuter force-pushed the fix/4324-fix-color-of-special-buttons branch from 811ecc5 to d6fb961 Compare August 21, 2023 08:50

nickvergessen merged commit 4c2c53e into master Aug 21, 2023
37 checks passed

nickvergessen deleted the fix/4324-fix-color-of-special-buttons branch August 21, 2023 13:37

JuliaKirschenheuter mentioned this pull request Dec 15, 2023

[BITV]: adapt checkboxes colors in normal state on dark theme #42121

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change color of error and success #39391

Change color of error and success #39391

JuliaKirschenheuter commented Jul 14, 2023 •

edited

Loading

Pytal commented Jul 17, 2023

Pytal commented Jul 17, 2023

JuliaKirschenheuter commented Jul 19, 2023

Pytal commented Jul 20, 2023

JuliaKirschenheuter commented Jul 21, 2023

JuliaKirschenheuter commented Jul 24, 2023

JuliaKirschenheuter commented Jul 24, 2023

nimishavijay left a comment

JuliaKirschenheuter commented Aug 11, 2023

nimishavijay commented Aug 16, 2023

skjnldsv commented Aug 18, 2023

Change color of error and success #39391

Change color of error and success #39391

Conversation

JuliaKirschenheuter commented Jul 14, 2023 • edited Loading

Summary

Checklist

Pytal commented Jul 17, 2023

Pytal commented Jul 17, 2023

JuliaKirschenheuter commented Jul 19, 2023

Pytal commented Jul 20, 2023

JuliaKirschenheuter commented Jul 21, 2023

JuliaKirschenheuter commented Jul 24, 2023

JuliaKirschenheuter commented Jul 24, 2023

nimishavijay left a comment

Choose a reason for hiding this comment

JuliaKirschenheuter commented Aug 11, 2023

nimishavijay commented Aug 16, 2023

skjnldsv commented Aug 18, 2023

JuliaKirschenheuter commented Jul 14, 2023 •

edited

Loading