Skip to content

Fix primary element color variables - #33641

Merged
skjnldsv merged 3 commits into
masterfrom
bugfix/noid/primary-element-vars
Aug 26, 2022
Merged

Fix primary element color variables#33641
skjnldsv merged 3 commits into
masterfrom
bugfix/noid/primary-element-vars

Conversation

@juliusknorr

@juliusknorrjuliusknorr commented Aug 22, 2022

Copy link
Copy Markdown
Member

In order to have proper variables available to be used with a fallback for very bright colors, this adds the same set of color variables to the theme which based its calculation on the element color.

Required for nextcloud-libraries/nextcloud-vue#3070

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 🐘

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
juliusknorrforce-pushed the bugfix/noid/primary-element-vars branch from 9703868 to e85d191CompareAugust 26, 2022 11:32
@juliusknorr

Copy link
Copy Markdown
MemberAuthor

/compile

@skjnldsvskjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 26, 2022
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
juliusknorrforce-pushed the bugfix/noid/primary-element-vars branch from e85d191 to cc21ea6CompareAugust 26, 2022 12:48
@skjnldsv
skjnldsv merged commit efbe972 into masterAug 26, 2022
@skjnldsv
skjnldsv deleted the bugfix/noid/primary-element-vars branch August 26, 2022 17:59
@blizzzblizzz mentioned this pull request Aug 30, 2022
'--color-primary-element-light' => $colorPrimaryElementLight,
'--color-primary-element-light-text' => $colorPrimaryElement,
'--color-primary-element-light-hover' => $this->util->mix($colorPrimaryElementLight, $colorMainText, 90),
'--color-primary-element-text-dark' => $this->util->darken($this->util->invertTextColor($colorPrimaryElement) ? '#000000' : '#ffffff', 7),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @juliushaertl I see you added color-primary-element-text-dark which I cannot find anywhere before.
Was that a mistake? I see only one usage, but I don't think this is actually a necessary variable :)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't remember any use case either. Thinking about it maybe we should also start documenting some context to the variables on when to use?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's exactly how I encountered this variable 🙈

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

Labels

4. to releaseReady to be released and/or waiting for tests to finishbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@juliusknorr@ChristophWurst@GretaD@skjnldsv