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

Add theme setting #243

Merged
merged 17 commits into from
Jun 22, 2023
Merged

Add theme setting #243

merged 17 commits into from
Jun 22, 2023

Conversation

tanchekwei
Copy link
Contributor

@tanchekwei tanchekwei commented Jun 20, 2023

Add theme setting to setting page.

Allow user to choose between System, Light, and Dark mode.

No impact for Light theme as using current color scheme.

By default, use system theme when user first time launches the app.

If System mode, update theme when system theme changed.

Fix white screen on startup by delaying showing window only after URL is loaded.

image

@vercel
Copy link

vercel bot commented Jun 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
chatall ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 22, 2023 2:27pm

@tanchekwei
Copy link
Contributor Author

Change to draft, as found low contrast issue to be fixed

image

@sunner
Copy link
Member

sunner commented Jun 21, 2023

This is amazing! Can't wait for a merging!

There is a suggest that ChatMessage.vue always:

import "github-markdown-css/github-markdown-light.css";

But there is also a *-dark.css can be used.

Moreover, the following information may be helpful to you.

I imported github-markdown.css at first and found that the code style can changed lively when the system theme changed. So I enforce it to *-light.css (Yes, it is an ugly). I didn't dig deep to know why the css can know the system theme changing. But it is a good experience. FYI.

Thank you very much again!

@tanchekwei
Copy link
Contributor Author

This is amazing! Can't wait for a merging!

There is a suggest that ChatMessage.vue always:

import "github-markdown-css/github-markdown-light.css";

But there is also a *-dark.css can be used.

Moreover, the following information may be helpful to you.

I imported github-markdown.css at first and found that the code style can changed lively when the system theme changed. So I enforce it to *-light.css (Yes, it is an ugly). I didn't dig deep to know why the css can know the system theme changing. But it is a good experience. FYI.

Thank you very much again!

Thanks for the info.

I have successfully loaded github-markdown-light.css or dark.css depending on the data-theme set on the body tag using SCSS.

However, I am facing a roadblock when trying to dynamically load highlight.js/styles/github.css or github-dark.css used to highlight code block. When switching between themes, at best, it reloads once, but then the code block theme gets stuck unless the page is reloaded. I have tried combining it with the markdown style depending on the data-theme, but it doesn't work.

import hightlight js

I attempted to delete the existing CSS before reloading, but I encountered a "module not found" error, and I'm not sure why.

const themeModule = require.resolve('../node_modules/highlight.js/styles/github-dark.css');
delete require.cache[themeModule];
const newModule = require.resolve('../node_modules/highlight.js/styles/github.css');
import(newModule);

I also tried doing this in the main process, but it didn't work, no style show up in the document.

cssId = mainWindow.webContents.insertCSS(`@import url('${cssPath}');
window.webContents.removeInsertedCSS(cssId);

Then I tried looping through the <style> tags in the document and removing the existing highlight styles before adding the new ones. However, this method not working if building in release version.

Feel free to give suggestions, or do you think this issue is acceptable since we don't change the theme very frequently?

@tanchekwei tanchekwei marked this pull request as ready for review June 21, 2023 15:33
@tanchekwei
Copy link
Contributor Author

I think I will spend 1 more day on the issue, see if I can find any solution for that.

@sunner
Copy link
Member

sunner commented Jun 22, 2023

I think your work is good enough to present to users. Some little pities can be resolved as things going. What do you think?

@tanchekwei
Copy link
Contributor Author

I think your work is good enough to present to users. Some little pities can be resolved as things going. What do you think?

Okay, I got it. A good night's sleep helps. Kindly review and let me know if there is anything I can do.

@tanchekwei tanchekwei marked this pull request as ready for review June 22, 2023 03:35
Copy link
Member

@sunner sunner left a comment

Choose a reason for hiding this comment

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

I believe what you did is really great! Thank you very much!

But the markdown style is not effect. Could you fix it? Then we can merge & publish it to the users ASAP.

src/components/Messages/ChatMessage.vue Show resolved Hide resolved
src/App.vue Outdated Show resolved Hide resolved
@tanchekwei
Copy link
Contributor Author

Done @sunner.

@tanchekwei tanchekwei requested a review from sunner June 22, 2023 14:21
@sunner
Copy link
Member

sunner commented Jun 22, 2023

Thank you very much! The PR will be included in next release!

@tanchekwei tanchekwei deleted the dark-theme branch June 26, 2023 01:43
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.

2 participants