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.

Sign up for GitHub

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

Jump to bottom

fix(theming): adjust default --color-background-plain to new background #42456

Closed
wants to merge 1 commit into from
Closed

fix(theming): adjust default --color-background-plain to new background #42456

wants to merge 1 commit into from

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Dec 22, 2023 •
edited
Loading

Before --color-background-plain was the same as the default background image color. Now the image has changed.

It is noticeable on slow connection.

On the image for me color is #01679d while we use as primary #00679e, but 1/256 is not notable.

Probably there are more places that we need to adjust.

Screenshots

Before After
bg-before bg-after

Checklist

@ShGKme
fix(theming): adjust default --color-background-plain to new background
f9efd82 
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme added 3. to review Waiting for reviews feature: theming labels Dec 22, 2023
@ShGKme ShGKme added this to the Nextcloud 29 milestone Dec 22, 2023
@ShGKme ShGKme self-assigned this Dec 22, 2023
nimishavijay
nimishavijay approved these changes Dec 22, 2023
View reviewed changes
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.

Good catch!

@ShGKme
Copy link
Contributor Author

ShGKme commented Dec 22, 2023

Tests are related, theming services should be adjusted

@ShGKme ShGKme added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Dec 22, 2023
@ShGKme ShGKme marked this pull request as draft December 22, 2023 15:01
jancborchardt
jancborchardt approved these changes Dec 27, 2023
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Nice :) Is the color-background-plain needed anyway btw or is it now always color-primary-element?

@skjnldsv
Copy link
Member

You'll need to adjust testing
This variable is not random, it's generated fro the php Theming providers

@ShGKme ShGKme mentioned this pull request Jan 9, 2024
Merged
7 tasks
This was referenced Mar 12, 2024
Merged
Merged
Merged
@Altahrim Altahrim mentioned this pull request Mar 20, 2024
Merged
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
@susnux
Copy link
Contributor

susnux commented Jul 9, 2024

Conflicting files
apps/theming/css/default.css

@ShGKme
Copy link
Contributor Author

ShGKme commented Jul 15, 2024

Was fixed another way.

@ShGKme ShGKme closed this Jul 15, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jancborchardt jancborchardt jancborchardt approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@marcoambrosini marcoambrosini marcoambrosini approved these changes

@nimishavijay nimishavijay nimishavijay approved these changes

@susnux susnux Awaiting requested review from susnux

Assignees

@ShGKme ShGKme

Labels
2. developing Work in progress feature: theming pending dependency update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ShGKme @skjnldsv @susnux @jancborchardt @marcoambrosini @nimishavijay