Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change color for primary element on hover #39317

Merged
merged 1 commit into from Jul 25, 2023

Conversation

JuliaKirschenheuter
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter commented Jul 11, 2023

Summary

Fixes color for primary element on hover state nextcloud-libraries/nextcloud-vue#4193 (comment)

Checklist

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please adjust generation script

'--color-primary-element-hover' => $this->util->mix($colorPrimaryElement, $colorMainBackground, 60),

@JuliaKirschenheuter
Copy link
Contributor Author

@skjnldsv could you please help me how should this mix looks like?

@skjnldsv
Copy link
Member

skjnldsv commented Jul 11, 2023

@skjnldsv could you please help me how should this mix looks like?

I can't, i have no idea how your hex colour got chosen.

Usage is as follow

mix($hex, [$amount] ) : Allows you to mix another color to your color. Optionally you can decide to set the percent of second color or original color amount is ranged -100...0...100.

You have to pick the right amount to get as close as your #2c7cae as possible 🤷

@nimishavijay
Copy link
Member

I can't, i have no idea how your hex colour got chosen.

It is the slightly darker and bluer version of the previous hover color, but there was no logic per se. I can try and find another close color, how does the mix work?

@skjnldsv
Copy link
Member

I can't, i have no idea how your hex colour got chosen.

It is the slightly darker and bluer version of the previous hover color, but there was no logic per se. I can try and find another close color, how does the mix work?

Using 65 as a mix factor, we get #2c84b3. Which seems close to your #2c7cae @nimishavijay


You can try this here: https://play.phpsandbox.io/mexitek/phpcolors

<?php
use Mexitek\PHPColors\Color;

$color = new Color('#006AA3');
$background = new Color('#ffffff');

echo '#' . $color->mix($background, 65) . PHP_EOL;

@nimishavijay
Copy link
Member

Super nice! Thank you @skjnldsv :)

Unfortunately the color code you suggested does not have enough contrast against the main background color. However, using 75 as the mix factor, we get #1f7cae which is also basically the same and is accessible 🎉 so @JuliaKirschenheuter we can use #1f7cae as the color here :)

image

@skjnldsv
Copy link
Member

skjnldsv commented Jul 13, 2023

Unfortunately the color code you suggested does not have enough contrast against the main background color.

This is not a variable used for text btw, so contrast rules are a bit more flexible for big elements iirc :)
But all good for me with #1f7cae too

EDIT: just saw you actually used the color as background and compared with text, my bad! All good!! 🙇
EDIT2: wow, insane work on all the mockups/screenshots

@JuliaKirschenheuter
Copy link
Contributor Author

@nimishavijay @skjnldsv

After several tryings just saw you discussion.
Thanks a lot for your cooperation, i've taken a mix factor 75 and #1f7cae as color ;))

Copy link
Member

@nimishavijay nimishavijay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 :)

Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/4193-change-color-primary-hover branch from f30eebc to ddb808e Compare July 24, 2023 13:56
@skjnldsv skjnldsv merged commit 468fb5c into master Jul 25, 2023
39 checks passed
@skjnldsv skjnldsv deleted the fix/4193-change-color-primary-hover branch July 25, 2023 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BITV] Buttons accessibility improvements
7 participants