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

Replace accent color with grey for button hover background in Paella #1031

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

LukasKalbertodt
Copy link
Member

@LukasKalbertodt LukasKalbertodt commented Dec 11, 2023

We had problems with this as the automatically derived accent colors (dark and light version) look very different hue-wise. We thought about just making both configurable but I got an idea that I like better, I think: don't use the accent color for the hover effect. Instead, a grey color is used which makes hovering less "flashy", which is good IMO. The YouTube player for example only has very subtle hover effects. It does have a changing cursor, which Paella should get as well, but that's a different issue.

So yeah, I propose this as it's a really simple solution and I think it even looks better!
Opinions @oas777 @dagraf ?


Here are screenshots with a few different primary colors:

ETH Blau
image
image

ETH Grün
image
image

ETH Rot
image
image

ETH Purpur
image
image

Bern Blue
image
image

We had problems with this as the automatically derived accent colors
(dark and light version) look very different hue-wise. We thought about
just making both configurable but I got an idea that I like better, I
think: don't use the accent color for the hover effect. Instead, a grey
color is used which makes hovering less "flashy", which is good IMO.
The YouTube player for example only has very subtle hover effects. It
does have a changing cursor, which Paella should get as well, but that's
a different issue.
@github-actions github-actions bot temporarily deployed to test-deployment-pr1031 December 11, 2023 13:35 Destroyed
@github-actions github-actions bot temporarily deployed to test-deployment-pr1031 December 11, 2023 13:49 Destroyed
@dagraf
Copy link
Collaborator

dagraf commented Dec 11, 2023

I like this solution. Thx!

@oas777
Copy link
Collaborator

oas777 commented Dec 11, 2023

+1. Am I correct in assuming we still have two different "blues" in Tobira and Paella?

grafik

@LukasKalbertodt
Copy link
Member Author

Am I correct in assuming we still have two different "blues" in Tobira and Paella?

I'm not 100% sure what your asking:

  • Is the default blue of the Paella ETH skin different from the default blue of Tobira? -> Yes
  • Is the color used in the Paella Tobira player different from the color used outside the player in Tobira (e.g. links)? -> The lightness/brightness is different, but the hue and saturation is the same. But yes, currently there are 4 different brightness levels of blue in Tobira, the brightest of which is used in Paella.

But ok, good to hear you like the solution. Then I'm just waiting for a code review by Ole (currently sick, unfortunately).

@oas777
Copy link
Collaborator

oas777 commented Dec 12, 2023

  • But yes, currently there are 4 different brightness levels of blue in Tobira, the brightest of which is used in Paella.

Bummer. Is there a ticket for this already?

@LukasKalbertodt
Copy link
Member Author

Bummer. Is there a ticket for this already?

No, that is by design. We do need multiple brightness versions of the primary color. For example, the text color changes when hovering a main navigation item or a link. But there are many more examples of this and this was the case since the very beginning already. Needing multiple brightness levels of the primary color is not inherently bad. The problem with the Paella colors was that we created versions with two very different brightnesses (i.e. one dark, one bright) that were used right next to one another.

@oas777
Copy link
Collaborator

oas777 commented Dec 12, 2023

Is this related to what is being described at https://ethz.ch/staffnet/en/service/communication/corporate-design/colours.html under "Using different shades"?

@LukasKalbertodt
Copy link
Member Author

Exactly. Your CI has different shades for every color for the same reason that Tobira creates a few different shades of the primary color: just having one shade is just too limiting for UI design.

@oas777
Copy link
Collaborator

oas777 commented Dec 18, 2023

Great. So for https://video.test.tobira.ethz.ch/ to match the ETH CD, we simply select a specific shade for each element using color?

@LukasKalbertodt
Copy link
Member Author

Tobira will create the shades automatically. You only need to configure on shade (in a specific brightness range). Tobira does the rest for you.

@oas777
Copy link
Collaborator

oas777 commented Dec 18, 2023

Great. So if we configure #215CAF, it will use #4D7DBF, #7A9DCF, #A6BEDF, #D3DEEF, #E9EFF7, and #08407E accordingly / where needed? And what about Paella Player?

@LukasKalbertodt
Copy link
Member Author

Not exactly those brightness values/shades but still shades with the exact same color tone. And the Paella player in Tobira also uses that. Let's maybe talk about this in person on Wednesday, if you have further questions.

@owi92
Copy link
Member

owi92 commented Dec 19, 2023

I wonder why paella isn't using the exact same shade for progress bar and volume control. Didn't even notice this before. And I did have to check with a color picker, as they are very close to each other. But that is a paella thing, nothing to do with this PR.
Bildschirmfoto 2023-12-19 um 11 06 29

@owi92
Copy link
Member

owi92 commented Dec 19, 2023

Oh and I do like the hover solution as well, nice!

@owi92 owi92 merged commit 9c19d5c into elan-ev:master Dec 19, 2023
3 checks passed
@LukasKalbertodt LukasKalbertodt deleted the improve-paella-colors branch December 19, 2023 10:17
@oas777
Copy link
Collaborator

oas777 commented Dec 19, 2023

I wonder why paella isn't using the exact same shade for progress bar and volume control. Didn't even notice this before. And I did have to check with a color picker, as they are very close to each other. But that is a paella thing, nothing to do with this PR.

According to the conversation Lukas and I had with Valencia last week, this should become an issue here, so that "Tobira devs (Ole and myself) then figure out where the problem originates." Cf. #1039

@LukasKalbertodt
Copy link
Member Author

Correct, thanks for opening the issue!

@LukasKalbertodt LukasKalbertodt added the changelog:nope Not worth mentioning in the changelog label Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:nope Not worth mentioning in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants