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

Dark theme #212

Merged
merged 17 commits into from
Jan 4, 2024
Merged

Dark theme #212

merged 17 commits into from
Jan 4, 2024

Conversation

IraSoro
Copy link
Owner

@IraSoro IraSoro commented Dec 29, 2023

Closed #51

I added a dark theme. To do this, I rewrote the code a little to make it easier to continue adding new themes. It looks like this:
image

But I have a problem now. If the user has selected a dark theme and opens the application, then a highlight occurs, where the default theme (light) is first shown, then the dark one. I tried to fix this using useLayoutEffect, but it didn't work.

Also, I couldn’t adapt the ionSelector to the dark theme (I couldn’t change the background)

@IraSoro IraSoro added this to the v2.3.8 milestone Dec 29, 2023
@IraSoro IraSoro self-assigned this Dec 29, 2023
Copy link
Collaborator

@imblowfish imblowfish left a comment

Choose a reason for hiding this comment

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

I wrote some comments
About other problems, I think it should be ok to merge this theme as it now and add additional tasks to fix these bugs
Until then the dark theme may stay in beta

@@ -11,4 +11,15 @@ const cyclesInit: CyclesContextI = {
updateCycles: (_newCycles) => {},
};

interface ThemeContextI {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Usually interface name started with I, not ended

Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed

const { theme, updateTheme } = useContext(ThemeContext);

const themesList = [];
for (const item of ["basic", "dark"]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Call this theme dark(beta), because we have some problems with it for now

Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed

@IraSoro IraSoro merged commit 8cf4c73 into master Jan 4, 2024
2 checks passed
@IraSoro IraSoro deleted the 51-new-theme branch January 4, 2024 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add a new theme
2 participants