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

fix: improve constrast between background and buttons #5366

Merged
merged 1 commit into from
Dec 23, 2023

Conversation

RafaelsRamos
Copy link
Contributor

I tried many possible colours but ultimately decided to go for android:background. These were the reasons:

  • With android:background, the colour matches the status bar;
  • I think it is one of the most aesthetically appealing combinations;
  • YouTube follows a similar approach of a darker background with lighter buttons.

Here are the possible colours - Link to material-components colours

Below are all the combinations I checked. Please let me know if we should choose another colour.

colorSurfaceBright

colorSurfaceContainer

colorSurfaceContainerHigh

colorSurfaceContainerHighest

colorSurfaceDim

android:background

closes #5363

@IndusAryan
Copy link
Contributor

what is the sense of having pure amoled mode if the main fragment of player isnt pure black

@Bnyro
Copy link
Member

Bnyro commented Dec 21, 2023

what is the sense of having pure amoled mode if the main fragment of player isnt pure black

We will override the color we decide for in the end to become black in the amoled theme too ...
It's just about the normal theme here.

@Bnyro
Copy link
Member

Bnyro commented Dec 21, 2023

The reason for the background color changes were the design ideas in #5247.

I think that generally we should choose a dark background color, but I feel like android:colorBackground is still too bright compared to the draft in the linked discussion.

It seems like colorSurfaceDim is quite a good solution from what I've tried so far, but that's just my POV 🤔

@IndusAryan
Copy link
Contributor

IndusAryan commented Dec 21, 2023

the design is very good, but when the pure mode is on, the colors should amoled black everywhere, irrespective of the color theme, because it is meant for it. and those who dont want pure black can set turn it off.

plus i am unable to find the thread and recall what was your response to removal of vertical related items , the horizontal one looks ugly af and doesnt justify the guidelines of an ideal video streaming app.

@DanielProg39
Copy link

colorSurfaceDim and android:background have identical colour schemes in dark mode, which I like best, but the light mode of colorSurfaceDim is more appealing to me, so I prefer this option.

@RafaelsRamos
Copy link
Contributor Author

colorSurfaceDim was my second favourite.

If you guys think that approach is best let me know 👍

@DanielProg39
Copy link

DanielProg39 commented Dec 21, 2023

As I said, I like the light mode of colorSurfaceDim a bit more, but I only use the dark mode, so my thoughts on the light mode are not really appropriate, so it doesn't make much difference to me, I would be happy with either of these options :)

So in general, I think it's more up to the users of the light mode to decide which option is better. At the very least we can open a discussion about it (or just continue #5247) to get more attention and feedback.

@M00NJ
Copy link

M00NJ commented Dec 23, 2023

I'm the person who created the mock-ups for the new design. Thanks for the ping @RafaelsRamos .
Like it it was said android:colorBackground and colorSurfaceDim are the same in dark mode but the later will keep it's relative brightness in light mode, so it won't be the brightest thing on screen. This can make sense for a streaming app, though with colorSurfaceDim the status bar color won't be the same in light mode but with android:colorBackground it will. So for that reason I would stick with the default background color. However just like @DanielProg39 I don't use light mode, so my opinion shouldn't be weighed to much here.

@RafaelsRamos
Copy link
Contributor Author

Thank you for your input, M00NJ ! 🙌

After re-reading all comments, we have a tie between the 2. Because M00NJ created the mockups, I propose that he gets the highest weight in the discussion.

Please, let me know if you guys disagree.

@Bnyro Bnyro changed the title Improve constrast between background and buttons fix: improve constrast between background and buttons Dec 23, 2023
@Bnyro Bnyro merged commit 887d062 into libre-tube:master Dec 23, 2023
3 of 4 checks passed
@DanielProg39
Copy link

I also thought about android:colorBackground as an equal option because of the status bar color, but in the end I don't think that this really matters and has much impact (I think a brighter status bar won't be an issue at all for the vast majority of users), so I'd rather look at this from an aesthetic perspective.

That said, although I (aesthetically) like colorSurfaceDim a bit more, I'm fine with both options and I don't mind either of them being taken, so I'm OK with @RafaelsRamos' suggestion

@RafaelsRamos RafaelsRamos deleted the fix/5363 branch January 22, 2024 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[master] Buttons in player page almost have no background
5 participants