Add the ability to select custom themes in the settings dialog #4570

pajlada · 2023-04-21T21:46:15Z

Description

This loads themes (any .json file) from the /Themes directory on startup and lists those in the settings page.
The custom themes in that directory will have a Custom: prefix to show that they're custom.
In the settingsfile, we save the name of the theme (e.g. 'forsen', no more info).

When the theme is updated or on Chatterino startup, we find the best matching theme with that name, whether it's a built in theme or a custom theme.

There's currently no mechanism for making a theme that has the same name as a built in theme, and they still show up in the menu. IMO we should just not allow duplicate names for simplicity's sake.

There's a few things left to do, and a few things to discuss before taking this step, even though I still deem it to be "pre-production" even when this PR is merged in.

The changelog entry is left as a Dev entry on purpose since I don't think this is a feature that should be fully announced until there's more testing & better UX for users.

github-actions

clang-tidy made some suggestions

src/singletons/Theme.cpp

github-actions · 2023-04-21T22:16:20Z

src/singletons/Theme.cpp

 }

 }  // namespace

 namespace chatterino {

+const std::map<QString, ThemeDescriptor> Theme::builtInThemes{


warning: invalid case style for global constant 'builtInThemes' [readability-identifier-naming]

Suggested change

const std::map<QString, ThemeDescriptor> Theme::builtInThemes{

const std::map<QString, ThemeDescriptor> Theme::BUILT_IN_THEMES{

src/singletons/Theme.cpp

github-actions · 2023-04-21T22:16:20Z

src/singletons/Theme.cpp

 {
-    QFile file(getThemePath(this->themeName));
-    if (!file.open(QFile::ReadOnly))
+    QStringList themeNames;


warning: variable 'themeNames' is not initialized [cppcoreguidelines-init-variables]

Suggested change

QStringList themeNames;

QStringList themeNames = 0;

github-actions · 2023-04-21T22:16:20Z

src/singletons/Theme.hpp

 namespace chatterino {

 class WindowManager;

+struct ThemeDescriptor {


warning: constructor does not initialize these fields: path, custom [cppcoreguidelines-pro-type-member-init]

src/singletons/Theme.hpp:21:

- QString path; + QString path{};

src/singletons/Theme.hpp:23:

- bool custom; + bool custom{};

github-actions · 2023-04-21T22:16:21Z

src/singletons/Theme.hpp

 class Theme final : public Singleton
 {
 public:
-    Theme();
+    static const std::map<QString, ThemeDescriptor> builtInThemes;


warning: invalid case style for global constant 'builtInThemes' [readability-identifier-naming]

Suggested change

static const std::map<QString, ThemeDescriptor> builtInThemes;

static const std::map<QString, ThemeDescriptor> BUILT_IN_THEMES;

github-actions · 2023-04-21T22:16:21Z

src/singletons/Theme.hpp

+    static const std::map<QString, ThemeDescriptor> builtInThemes;
+
+    // The built in theme that will be used if some theme parsing fails
+    static const ThemeDescriptor fallbackTheme;


warning: invalid case style for global constant 'fallbackTheme' [readability-identifier-naming]

Suggested change

static const ThemeDescriptor fallbackTheme;

static const ThemeDescriptor FALLBACK_THEME;

src/widgets/settingspages/GeneralPage.cpp

src/singletons/Theme.cpp

dnsge · 2023-05-01T20:10:53Z

It'd be great if there was hot reloading of the current theme when the JSON file is modified. This would make creating themes and trying out different colors easy (compared to what I imagine is having to restart Chatterino after every change).

There appears to be a Qt class that would make detecting the file change easy (in theory, barring os-specific annoyances): https://doc.qt.io/qt-6/qfilesystemwatcher.html. I haven't dug through the Theme code that much though, so it's possible that this is more difficult than it sounds.

pajlada · 2023-05-01T20:18:52Z

It'd be great if there was hot reloading of the current theme when the JSON file is modified. This would make creating themes and trying out different colors easy (compared to what I imagine is having to restart Chatterino after every change).

I would like that type of functionality in a V2/V3 kind of thing instead of in the initial release.
Initially I'd like this in a nightly release without any commitments to how/where the themes are located so it can be tested, then we look into specifics for how themes are worked on by users.

I could see a world where theme creation isn't tested in Chatterino itself, but rather a web UI. Or there's a /reload-theme command to do the reloading.

Nerixyz · 2023-05-01T20:23:56Z

There appears to be a Qt class that would make detecting the file change easy (in theory, barring os-specific annoyances): https://doc.qt.io/qt-6/qfilesystemwatcher.html. I haven't dug through the Theme code that much though, so it's possible that this is more difficult than it sounds.

This should be the way to go. I had a small implementation that used the file system watcher, and it worked well (Windows & VSCode).

These are only loaded on startup To make this hot reload, we'd need a file watcher or have it just refresh any time the settings page is opened

pajlada · 2023-05-19T11:46:26Z

github-actions

clang-tidy made some suggestions

github-actions · 2023-05-19T12:27:34Z

src/singletons/Theme.cpp

 }

 }  // namespace

 namespace chatterino {

+const std::vector<ThemeDescriptor> Theme::builtInThemes{


warning: invalid case style for global constant 'builtInThemes' [readability-identifier-naming]

Suggested change

const std::vector<ThemeDescriptor> Theme::builtInThemes{

const std::vector<ThemeDescriptor> Theme::BUILT_IN_THEMES{

github-actions · 2023-05-19T12:27:34Z

src/singletons/Theme.cpp

 {
-    QFile file(getThemePath(this->themeName));
-    if (!file.open(QFile::ReadOnly))
+    std::vector<std::pair<QString, QVariant>> packagedThemes;


warning: variable 'packagedThemes' is not initialized [cppcoreguidelines-init-variables]

Suggested change

std::vector<std::pair<QString, QVariant>> packagedThemes;

std::vector<std::pair<QString, QVariant>> packagedThemes = 0;

github-actions · 2023-05-19T12:27:35Z

src/singletons/Theme.cpp

+    }
+}
+
+std::optional<ThemeDescriptor> Theme::findThemeByKey(const QString &key)


warning: method 'findThemeByKey' can be made static [readability-convert-member-functions-to-static]

Suggested change

std::optional<ThemeDescriptor> Theme::findThemeByKey(const QString &key)

static std::optional<ThemeDescriptor> Theme::findThemeByKey(const QString &key)

github-actions · 2023-05-19T12:27:35Z

src/singletons/Theme.hpp


 namespace chatterino {

 class WindowManager;

+struct ThemeDescriptor {


warning: constructor does not initialize these fields: key, path, name [cppcoreguidelines-pro-type-member-init]

src/singletons/Theme.hpp:21:

- QString key; + QString key{};

src/singletons/Theme.hpp:25:

- QString path; + QString path{};

src/singletons/Theme.hpp:28:

- QString name; + QString name{};

github-actions · 2023-05-19T12:27:35Z

src/singletons/Theme.hpp

 class Theme final : public Singleton
 {
 public:
-    Theme();
+    static const std::vector<ThemeDescriptor> builtInThemes;


warning: invalid case style for global constant 'builtInThemes' [readability-identifier-naming]

Suggested change

static const std::vector<ThemeDescriptor> builtInThemes;

static const std::vector<ThemeDescriptor> BUILT_IN_THEMES;

github-actions · 2023-05-19T12:27:35Z

src/widgets/settingspages/GeneralPageView.hpp

+    template <typename T>
+    ComboBox *addDropdown(
+        const QString &text,
+        const std::vector<std::pair<QString, QVariant>> &items,


warning: all parameters should be named in a function [readability-named-parameter]

Suggested change

const std::vector<std::pair<QString, QVariant>> &items,

const std::vector<std::pair<QString, QVariant> /*unused*/> &items,

github-actions · 2023-05-19T12:27:35Z

src/widgets/settingspages/GeneralPageView.hpp

+        std::function<T(DropdownArgs)> setValue, QString toolTipText = {},
+        const QString &defaultValueText = {})
+    {
+        auto *combo = this->addDropdown(text, {}, std::move(toolTipText));


warning: variable 'combo' is not initialized [cppcoreguidelines-init-variables]

Suggested change

auto *combo = this->addDropdown(text, {}, std::move(toolTipText));

auto *combo = nullptr = this->addDropdown(text, {}, std::move(toolTipText));

github-actions bot reviewed Apr 21, 2023

View reviewed changes

Nerixyz reviewed May 1, 2023

View reviewed changes

src/singletons/Theme.cpp Show resolved Hide resolved

pajlada added this to the Post 2.4.4 milestone May 8, 2023

pajlada force-pushed the feat/custom-themes-v2 branch from 3357fe1 to bd1881b Compare May 19, 2023 09:52

pajlada added the no changelog entry needed label May 19, 2023

pajlada added 3 commits May 19, 2023 13:35

Define & create themes directory

6f4c82d

Load custom themes from the Themes directory

96ec684

These are only loaded on startup To make this hot reload, we'd need a file watcher or have it just refresh any time the settings page is opened

Make it so when custom themes can have the same name as built-in themes

e7cfba8

pajlada force-pushed the feat/custom-themes-v2 branch from bd1881b to e7cfba8 Compare May 19, 2023 11:35

pajlada added 6 commits May 19, 2023 13:38

Add changelog entry

ec02bad

Change theme change from debug to info

b8806e2

remove map include

a0c54a2

include qstring

5b8252f

Remove explicit return values

56e5a9f

remove tooltip text

8bca0a4

pajlada marked this pull request as ready for review May 19, 2023 11:45

pajlada enabled auto-merge (squash) May 19, 2023 11:47

pajlada merged commit 5d0bdc1 into master May 19, 2023

pajlada deleted the feat/custom-themes-v2 branch May 19, 2023 12:26

github-actions bot reviewed May 19, 2023

View reviewed changes

ilya-zlobintsev mentioned this pull request Jan 30, 2024

Add developer_name appdata field ilya-zlobintsev/chatterino2#3

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the ability to select custom themes in the settings dialog #4570

Add the ability to select custom themes in the settings dialog #4570

pajlada commented Apr 21, 2023 •

edited

Loading

github-actions bot left a comment

github-actions bot Apr 21, 2023

github-actions bot Apr 21, 2023

github-actions bot Apr 21, 2023

github-actions bot Apr 21, 2023

github-actions bot Apr 21, 2023

dnsge commented May 1, 2023

pajlada commented May 1, 2023

Nerixyz commented May 1, 2023

pajlada commented May 19, 2023

github-actions bot left a comment

github-actions bot May 19, 2023

github-actions bot May 19, 2023

github-actions bot May 19, 2023

github-actions bot May 19, 2023

github-actions bot May 19, 2023

github-actions bot May 19, 2023

github-actions bot May 19, 2023

	const std::map<QString, ThemeDescriptor> Theme::builtInThemes{
	const std::map<QString, ThemeDescriptor> Theme::BUILT_IN_THEMES{

	static const std::map<QString, ThemeDescriptor> builtInThemes;
	static const std::map<QString, ThemeDescriptor> BUILT_IN_THEMES;

	static const ThemeDescriptor fallbackTheme;
	static const ThemeDescriptor FALLBACK_THEME;

	const std::vector<ThemeDescriptor> Theme::builtInThemes{
	const std::vector<ThemeDescriptor> Theme::BUILT_IN_THEMES{

	std::vector<std::pair<QString, QVariant>> packagedThemes;
	std::vector<std::pair<QString, QVariant>> packagedThemes = 0;

	std::optional<ThemeDescriptor> Theme::findThemeByKey(const QString &key)
	static std::optional<ThemeDescriptor> Theme::findThemeByKey(const QString &key)

	static const std::vector<ThemeDescriptor> builtInThemes;
	static const std::vector<ThemeDescriptor> BUILT_IN_THEMES;

	const std::vector<std::pair<QString, QVariant>> &items,
	const std::vector<std::pair<QString, QVariant> /unused/> &items,

	auto *combo = this->addDropdown(text, {}, std::move(toolTipText));
	auto *combo = nullptr = this->addDropdown(text, {}, std::move(toolTipText));

Add the ability to select custom themes in the settings dialog #4570

Add the ability to select custom themes in the settings dialog #4570

Conversation

pajlada commented Apr 21, 2023 • edited Loading

Description

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Apr 21, 2023

Choose a reason for hiding this comment

github-actions bot Apr 21, 2023

Choose a reason for hiding this comment

github-actions bot Apr 21, 2023

Choose a reason for hiding this comment

github-actions bot Apr 21, 2023

Choose a reason for hiding this comment

github-actions bot Apr 21, 2023

Choose a reason for hiding this comment

dnsge commented May 1, 2023

pajlada commented May 1, 2023

Nerixyz commented May 1, 2023

pajlada commented May 19, 2023

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot May 19, 2023

Choose a reason for hiding this comment

github-actions bot May 19, 2023

Choose a reason for hiding this comment

github-actions bot May 19, 2023

Choose a reason for hiding this comment

github-actions bot May 19, 2023

Choose a reason for hiding this comment

github-actions bot May 19, 2023

Choose a reason for hiding this comment

github-actions bot May 19, 2023

Choose a reason for hiding this comment

github-actions bot May 19, 2023

Choose a reason for hiding this comment

pajlada commented Apr 21, 2023 •

edited

Loading