Add dark theme

[grandeljay]

In an attempt to finish what was started at #1436

I initially said I would attempt to continue where @AhmedEid3 left off but I was struggling a bit so I decided to start over. So far I have just swapped the default colors with the inverted ones but the result looks promising in the /examples/theming.html

Feel free to already let me know your thoughts

[lubber-de]

Please remove the /dist folder changes from your PR for easier review. Thanks

[lubber-de]

@grandeljay
TL;DR: The /dist folder should not be part of your PR, not even as a delete

You have now completely deleted the /dist folder. So if we would merge it, the /dist folder gets deleted.
What i meant is to remove the /dist folder from your commit only, so the /dist folder stays untouched.
I suggest to

• save/backup your changes in the /src folder somewhere
• reset your branch to the develop branch (yes all changes are lost..but you kept the /src changes in the last step)
• apply/copy your previously saved changes files to the /src folder again
• git push --force

[grandeljay]

@lubber-de I've copied the /dist directory from the upstream/develop branch and most of the changes seem to have disappeared. Some files seem to have been added though (like flyout). Why is that? Shouldn't it all be unchanged now? Did I clone the wrong branch? It still says Version 2.8.8, shouldn't it be 2.9.0 beta or something like that?

[lubber-de]

The /dist folder from the develop branch does not contain the flyout component (yet), because the dist folder will only be updated on a new release.
But the current develop branch already has the flyout components inside the /src folder. So it seems you have build fomantic locally yourself (or did an npm install) and afterwards copied the /dist folder from there. thats why flyout is now "new" to the /dist folder in comparison to the develop branch.

Same for the /dist/themes folder. Please don't commit them

I again suggest to reset your branch to the current develop branch and only do the changes inside the /src folder.
Of course you can build locally however you like, just make sure on commit to ignore the /dist folder.

[lubber-de]

It still says Version 2.8.8, shouldn't it be 2.9.0 beta or something like that?

it is still 2.8.8 inside the develop branch. 2.9.0 beta-xxxxx is set on the nightly builds only.
We'll update the version inside package.json on a new release only

[grandeljay]

Ok, thanks. All the changes should be gone now.

If I remember correctly you said there is no way to make this default-dark theme work automatically with prefers-color-scheme: dark?

I'd have to create a CSS file and import it, like you mentioned here. If that's the case, maybe I shouldn't be creating the dark theme like I have now and instead go for a CSS file

[jamessampford]

I know there are a few considerations around implementing a dark mode, but just to add my 2 cents…

Maintaining
Will both default and default-dark both be kept up to date?
Should both have a common file containing variables that related to positioning/sizing, and separate ones for colour, to ensure consistency for positioning/sizing?

Using Dark Mode
What if we want to toggle between light and dark mode manually, rather than relying on the OS setting?
Could it be that a semantic.dark.css files is generated within the default folder with just what is needed for dark? If this, then should we also have a semantic.light.css file with anything relating to anything related to anything but colour only in semantic.css? This may help with maintenance
Could, say, both prefers-color-scheme: dark as well as either body.inverted or body[data-mode=“dark”] be used? The latter could possibly also go to light mode by specifying light

[grandeljay]

Ok, I've been giving this a lot of thought.

There is no elegant, low maintenance way of doing this so far I am trying something like this:

/**
 * Dark mode
 *
 * Adds .inverted to all components.
 */
function ready() {
    const darkModePreference = window.matchMedia("(prefers-color-scheme: dark)");

    if (darkModePreference.matches) {
        const usedComponents = ["container", "grid", "button", "calendar", "card", "checkbox", "dimmer", "divider", "dropdown", "form", "header", "icon", "image", "input", "label", "list", "loader", "menu", "message", "modal", "placeholder", "popup", "progress", "segment", "sidebar", "statistics", "step", "tab", "table", "text", "toast", "api", "transition"];

        usedComponents.forEach(usedComponent => {
            let uiComponents = document.querySelectorAll('.ui.' + usedComponent);

            uiComponents.forEach(component => {
                if (component.classList.contains('inverted')) {
                    component.classList.remove('inverted');
                } else {
                    component.classList.add('inverted');
                }
            });
        });
    }
}

document.addEventListener("DOMContentLoaded", ready);

It's quite primitive but it allows me to use a dark mode without hard-coding a bunch of CSS markup. This also scales well with fomantic updates/changes.

90% looks good to me, I can do some manual tweaks in my own CSS then.

Thoughts?

[lubber-de]

Nice approach! However, there are still many components, which do not have an inverted variant which possibly results into remaining visual issue when also the background is switched to something like black.

[grandeljay]

Thanks!

Would you still consider this? IMO it's the smartest solution yet. It would theoretically work with all themes!

However, there are still many components, which do not have an inverted variant

Adding missing inverted variant could be done in FUI to solve this. I think that would be the "proper" way of doing things in this case.

[jamessampford]

Clever - could potentially also use on the docs to toggle examples between normal and inverted (fomantic/Fomantic-UI-Docs#339) this could then identify where inverted may be missing or where this may need tweaking further (such as black backgrounds for segments, as lubber pointed out, and think about what happens if a child node contains another coloured item eg. A white segment within a black one)

[lubber-de]

Would you still consider this? IMO it's the smartest solution yet. It would theoretically work with all themes!

For a proper "default dark" theme switchable feature, i believe we really have to rework all components and move many switchable color values to css variables. Then provide a core css file and 2 "colors only" default "themes" which contain color differences only. Then it is only a matter of adding/removing one single class like "dark" to the body rather than the need to iterate through the whole DOM and switching lots of classes (what happens on dynamically changed DOM nodes?)

But, for the time being and the current state of FUI themes, your approach seems valid "enough" for whom it works out nicely.

Whenever we would start telling people this is the "official" way to do it, they will start claiming what combinations will not look nice, and so on.

Adding missing inverted variant could be done in FUI to solve this. I think that would be the "proper" way of doing things in this case.

Well, it is most probably more than just adding more inverted variants. Using your approach would mean , all possible combinations have to visually work out just as nice as the current default theme only does.
And if people are using inverted variants inside their "light" theme, toggling them has to look as good as before.