Skip to content

Conversation

@CarlSchwan
Copy link
Member

Fix #34514

@CarlSchwan CarlSchwan added this to the Nextcloud 25 milestone Oct 11, 2022
@CarlSchwan CarlSchwan requested a review from a team October 11, 2022 21:16
@CarlSchwan CarlSchwan self-assigned this Oct 11, 2022
@CarlSchwan CarlSchwan requested review from Pytal, artonge and szaimen and removed request for a team October 11, 2022 21:16
@CarlSchwan CarlSchwan added the 3. to review Waiting for reviews label Oct 11, 2022
@blizzz blizzz mentioned this pull request Oct 11, 2022
2 tasks
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Oct 11, 2022
@blizzz
Copy link
Member

blizzz commented Oct 11, 2022

master is 26

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

I think we need to fix fhe variable for these themes. See

$colorMainBackgroundRGB = join(',', $this->util->hexToRGB($colorMainBackground));
and
'--color-main-background-rgb' => $colorMainBackgroundRGB,

@CarlSchwan
Copy link
Member Author

I think we need to fix fhe variable for these themes. See

@szaimen I'm not sure I understand correctly what you mean here. What is the problem with this variables?

@szaimen
Copy link
Contributor

szaimen commented Oct 12, 2022

I think we need to fix fhe variable for these themes. See

@szaimen I'm not sure I understand correctly what you mean here. What is the problem with this variables?

in high contrast mode dark is --color-main-background-rgb 23,23,23 but it should be 0,0,0

@CarlSchwan
Copy link
Member Author

in high contrast mode dark is --color-main-background-rgb 23,23,23 but it should be 0,0,0

Contrast is already 17 this is already way higher than the required contrast for AAA. High contrast theme doesn't mean black on white theme ;)

@szaimen
Copy link
Contributor

szaimen commented Oct 12, 2022

Contrast is already 17 this is already way higher than the required contrast for AAA. High contrast theme doesn't mean black on white theme ;)

fine but it is still wrong. will push the change to the branch shortly

@szaimen
Copy link
Contributor

szaimen commented Oct 12, 2022

Contrast is already 17 this is already way higher than the required contrast for AAA. High contrast theme doesn't mean black on white theme ;)

fine but it is still wrong. will push the change to the branch shortly

done with f1b3842

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

lgtm now but didn't test

@szaimen szaimen merged commit cbe228a into master Oct 12, 2022
@szaimen szaimen deleted the fix/theming-highcontrast-translucency-off branch October 12, 2022 22:56
@szaimen
Copy link
Contributor

szaimen commented Oct 12, 2022

/backport to stable25

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.

header transparent in files app in high-contrast mode

5 participants