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

Updated theme to work via css variables #433

Merged
merged 13 commits into from
Aug 4, 2023

Conversation

hexnickk4997
Copy link
Contributor

@hexnickk4997 hexnickk4997 commented Aug 1, 2023

Description

Also fixes UI-920

  • moved ContentTheme & ThemeToggler components inside theme folder since they are directly related to theme functionality
  • There is no ThemeProvider, which accepts any theme anymore
  • Moved themes to singe file, it's easier to work with them, especially to set up custom selectors

Demo

Content Theme

image image

Theme Providers

image image

Theme Toggler

image image

Cookie is also supported
image

Manually changed to light
image

System theme also supported

image

@hexnickk4997 hexnickk4997 requested review from a team as code owners August 1, 2023 14:47
Comment on lines +189 to +190
--lido-rgb-errorHover: 212, 76, 77;
--lido-color-errorContrast: #fff;
Copy link
Contributor

Choose a reason for hiding this comment

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

-rgb- is used in some of widgets for re-using color but making it transparent. please, preserve it in new version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, will add

}> = ({ jsStyleScript = true, cssFont = false }) => (
<>
{jsStyleScript ? <ScriptThemeValue /> : null}
{cssFont ? <Fonts /> : null}
Copy link
Contributor

Choose a reason for hiding this comment

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

why it is false by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,44 @@
const getDefinedVariables = async () => {
const content = await fs.readFile('./packages/theme/theme.css', 'utf-8')
Copy link
Contributor

Choose a reason for hiding this comment

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

leave annotation in theme.css that it is used here and should not be moved or renamed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

also, please add comment for this file - what is it doing?

process.exit(1)
}

void main()
Copy link
Contributor

Choose a reason for hiding this comment

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

top-level await should be already supported, no need to wrap

@hexnickk4997 hexnickk4997 merged commit eb82ed6 into v4 Aug 4, 2023
@hexnickk4997 hexnickk4997 deleted the feature/ui-866-migrate-theme branch August 4, 2023 17:15
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.

3 participants