vkurko / calendar

Full-sized drag & drop JavaScript event calendar with resource & timeline views
https://vkurko.github.io/calendar/
MIT License
969 stars 82 forks source link

Dark theme #166

Closed ademaro closed 8 months ago

ademaro commented 10 months ago

Styles in the project are not the most convenient for refactoring and support. It would be good to rewrite the layout and styles with a normal methodology. But perhaps there are reasons for that. That's why I made the first realization of the dark theme without deep rewriting.

Let's discuss how you like it in terms of implementation. The switch, I think, should be rewritten in Svetle, I haven't worked with it.

You can see a live example here: https://jokerinteractive.github.io/calendar/

If you open the browser inspector, you can play around with the base hue. To do this, change the value of the variable --hue-primary Just below you can play with the saturation value --saturation-primary

vkurko commented 10 months ago

Thanks for your contribution. I have 2 comments:

  1. Since the calendar is a library that can be used in a variety of environments, all classes and variables must at least be prefixed with "ec-".
  2. I don't quite understand the use of the extra body-light and body-dark classes that are set by the extra JS code in your demo. I think the calendar should work out of the box without additional JS.
vkurko commented 10 months ago

Ok. Regarding the second point, I guess that this was done in order to be able to force the calendar to switch to light or dark mode.

ademaro commented 10 months ago
  1. Since the calendar is a library that can be used in a variety of environments, all classes and variables must at least be prefixed with "ec-".

Got it, makes sense, I'll rewrite it.

  1. I don't quite understand the use of the extra body-light and body-dark classes that are set by the extra JS code in your demo. I think the calendar should work out of the box without additional JS.

I implemented as an example through localStorage, but you can rewrite it on the backend in a personal project (if there is some authorization, for example, through sessions), through cookies or something else. I don't know yet how best to formalize it for this project.

vkurko commented 10 months ago

Got it, makes sense, I'll rewrite it.

Any progress on this?

ademaro commented 10 months ago

Been very busy last week, will try to close the issue one of these days.

But I'd still like to hear your thoughts on the theme switcher: are we going to keep it as part of the calendar or will we put it out as a piece of demo (where everyone can implement it in their projects as they want)?

vkurko commented 10 months ago

I would probably make the theme switch through a separate css class for the calendar container (you now have body-light/body-dark for this). That is, if someone wants to force a dark theme, then they will add a class, let's say ec-dark, to the container in which the calendar is initialized. This is all just to minimize the impact on the environment.

Therefore, you also need to avoid styling the body:

body {
  color: var(--theme-fg);
  background-color: var(--theme-bg);
}
ademaro commented 8 months ago

@vkurko Is there anything else I need to modify to include this PR in the main branch?

vkurko commented 8 months ago

@vkurko Is there anything else I need to modify to include this PR in the main branch?

Sorry, I was waiting for your final confirmation that everything was ready. I looked at the current version and there are still global styles affecting the whole document. For example:

body {
  color: var(--ec-theme-fg);
  background-color: var(--ec-theme-bg);
}
:root {
  ...
  color-scheme: light dark;
  ...
}

The Event Calendar is a library that is embedded in someone else's page. It should not affect the display of other page elements that are not related to the calendar in any way.

Basically I can make the corrections myself. I'll do it when I have free time.

And thank you very much for your contribution to the dark theme. I really appreciate it.

ademaro commented 8 months ago

Yes, you are right, I removed the global styles. Also double-checked with the latest version, it seems to be ok now.

vkurko commented 8 months ago

Ok, I'm merging this PR. But before the release I still want to change the HSL colors you got to ones that are more similar to the current colors.

Thank you for your contribution :smile:

vkurko commented 8 months ago

@ademaro I finally found the time to complete the dark theme.

I made some changes - sorry if I violated any of your initial ideas :smile:. If you have any comments, please let me know.

The dark theme is released in v2.4.0.

ademaro commented 8 months ago

Looks good! Thanks)