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

removed white background color from controls as this makes it almost … #1002

Closed
wants to merge 1 commit into from

Conversation

Erdbeergeist
Copy link

removed the white background color from the control as this makes it impossible to see the controls and the artist/album name see below:
image

…impossible to see the controls and the artist/album name
@paulijar
Copy link
Collaborator

We can't make it like that because then the app would look like this on NC25 with the default theme:
image
That is, the controls pane background will be completely transparent.

I guess you are using some 3rd party theme, which one? Maybe the fix should go to the theme. Or then we need to figure out a more intelligent way to do this.

Does the color of the control pane work nicely on that theme if you declare background-color: var(--color-main-background-translucent); for it? But even if this works, this is not yet a complete solution because this will not work on any version of ownCloud.

@Erdbeergeist
Copy link
Author

Erdbeergeist commented Aug 20, 2022

Ok I see your point, the theme is breeze dark https://github.com/mwalbeck/nextcloud-breeze-dark. Setting background-color: var(--color-main-background-translucent);
seems to work well. I have found that there is actually a custom styling option in the theme so using that to override the default background color works well too. Maybe this is not an issue, but then in other themes this might not work. Sorry for my ignorance but can you elaborate why your suggested fix wont work on all versions of ownCloud. Thank you !

@paulijar
Copy link
Collaborator

Sorry for my ignorance but can you elaborate why your suggested fix wont work on all versions of ownCloud.

That's because ownCloud doesn't declare those color properties, that's a Nextcloud-specific thing. However, I just learned that the css var() syntax allows giving a default value for the cases where the named property is not found. So we could use background-color: var(--color-main-background-translucent, rgba(255, 255, 255, .95));. Then, the theming on Nextcloud should work as intended without breaking the ownCloud default theme.

paulijar added a commit that referenced this pull request Aug 20, 2022
When we use the common color constants, we don't need to define as many
exceptions for the built-in dark theme of Nextcloud. Similarly, 3rd party
themes will have easier time as every element doesn't need its own custom
rule.

Note that the used color constant `color-main-background-translucent` is not
available on any version of ownCloud. To mitigate this, the hard-coded
translucent white color is used as a fallback color.

refs #1002
@paulijar
Copy link
Collaborator

I now committed a change like described above. In addition to the controls pane, I applied it on the background of the "scanning"/"no content available" banner as well as on the "embedded player" used within the Files app. The Breeze Dark theme already seemed to have an overriding rule for the embedded player background which is why no problem was seen there. But for the "scanning" banner, it uses the Music app's color definition.

@Erdbeergeist
Copy link
Author

perfect, thank you kindly :)

@paulijar
Copy link
Collaborator

The fix has been now released in Music v1.7.0.

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.

None yet

2 participants