Skip to content

Conversation

@PVince81
Copy link
Member

Make it the same grey like the stripe background and adjust text color
to be readable.

Fixes #4388

image

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

Now that I see it I think that it should be a darker gray and the text should stay white.
Maybe var(--color-loading-dark)??

@PVince81
Copy link
Member Author

do we have a darker grey somewhere in the presets or should I invent a value ?

@PVince81 PVince81 force-pushed the bugfix/4388/fix-empty-call-background branch from eb17d94 to e1ee2f2 Compare October 19, 2020 14:52
@PVince81
Copy link
Member Author

Here we go:
image

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

👍

Make the background grey and adjust text color to be readable.

Signed-off-by: Vincent Petry <[email protected]>
@PVince81 PVince81 force-pushed the bugfix/4388/fix-empty-call-background branch from e1ee2f2 to 4e50dfe Compare October 19, 2020 15:20
@PVince81 PVince81 merged commit f0e7a04 into master Oct 19, 2020
@PVince81 PVince81 deleted the bugfix/4388/fix-empty-call-background branch October 19, 2020 16:25
}
h2, p {
color: var(--color-primary-text);
Copy link
Member

Choose a reason for hiding this comment

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

You want white, not primary text, otherwise you have black text on dark grey background if your primary theming color is bright

Copy link
Member Author

Choose a reason for hiding this comment

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

should I hardcode white here ? then maybe I need to hardcode the background color as well instead of --color-loading-dark, if we agree that this is not something we want to theme.

as far as I can see here, only primary is defined: https://github.com/nextcloud/nextcloud-vue/blob/master/styleguide/assets/variables.css but if one would want to invert the colors they'd need to override "color-background-dark" with a light color ? or do we need a new primary-text-background color ?

Copy link
Member

@nickvergessen nickvergessen Oct 19, 2020

Choose a reason for hiding this comment

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

We dont want the screen to be blue or something so we should not use anything related to primary.
Also we dont want it to flip ot white with dark mode, so mostlikely the loading color is wrong too.

But can be tested easily, just search for "accessibility" in your dev nextcloud, turn on dark mode, search for theming and use "#ffffff" as primary color and then start a call.

Expected result is a dark screen with readable font color

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

it seems that whatever combination of variables I pick, it cannot look good in both dark mode and regular mode

so if we anyway don't want to invert the colors I'll hardcode white and dark grey in this view

Copy link
Member Author

Choose a reason for hiding this comment

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

=> #4416

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Black background for "waiting for others"

4 participants