Icon themes: Active-Directory (line, duotone, color) #425

LenkaDEA · 2023-11-15T14:39:39Z

Added icon themes: Active-Directory-line, Active-Directory-duotone, Active-Directory-color
Icon themes are set separately: https://github.com/LenkaDEA/admc_icons/tree/themes

Samael340 · 2023-11-22T11:27:33Z

src/admc/console_impls/query_folder_impl.cpp

@@ -341,7 +342,10 @@ void console_query_tree_init(ConsoleWidget *console) {
    const QList<QStandardItem *> root_row = console->add_scope_item(ItemType_QueryFolder, QModelIndex());
    auto root = root_row[0];
    root->setText(QCoreApplication::translate("query", "Saved Queries"));
-    root->setIcon(QIcon::fromTheme("folder"));
+
+    const QIcon create_query_folder_icon = g_icon_manager->get_object_icon("folder-query");


It is not related to the commit topic. Better move to another commit or create refactor branch

Samael340 · 2023-11-22T11:27:55Z

src/admc/console_impls/query_folder_impl.cpp

@@ -533,7 +537,9 @@ QModelIndex console_query_folder_create(ConsoleWidget *console, const QString &n
 void console_query_folder_load(const QList<QStandardItem *> &row, const QString &name, const QString &description) {
    QStandardItem *main_item = row[0];
    main_item->setData(description, QueryItemRole_Description);
-    main_item->setIcon(QIcon::fromTheme("folder"));
+
+    const QIcon create_query_folder_icon = g_icon_manager->get_object_icon("folder-query");


It is not related to the commit topic. Better move to another commit or create refactor branch

Samael340 · 2023-11-22T11:28:07Z

src/admc/console_impls/query_item_impl.cpp

@@ -68,7 +67,9 @@ void QueryItemImpl::fetch(const QModelIndex &index) {
    // NOTE: reset icon and tooltip in case query is in the
    // "out of date" state
    QStandardItem *item = console->get_item(index);
-    item->setIcon(QIcon::fromTheme(query_item_icon));
+
+    const QIcon create_query_item = g_icon_manager->get_object_icon("query-item");


It is not related to the commit topic. Better move to another commit or create refactor branch

Samael340 · 2023-11-22T11:35:19Z

src/admc/icon_manager/icon_manager.cpp

@@ -11,28 +12,54 @@ IconManager::IconManager()
 {
 }

-void IconManager::init()
+void IconManager::init(QString icon_theme)


Init method is used for object initialization. In my opinion its better to create another function like set_theme() to change themes.

Samael340 · 2023-11-22T11:41:52Z

src/admc/settings.h

@@ -99,6 +99,7 @@ DEFINE_SETTING(SETTING_create_shared_folder_dialog_geometry);
 DEFINE_SETTING(SETTING_create_contact_dialog_geometry);
 DEFINE_SETTING(SETTING_find_policy_dialog_geometry);
 DEFINE_SETTING(SETTING_time_span_attribute_dialog_geometry);
+DEFINE_SETTING(SETTING_icons_theme_geometry);


I think better rename it to "SETTING_icons_theme" and move it to "Other" settings. This does not apply to geometry settings.

Samael340 · 2023-11-22T11:56:17Z

src/admc/main_window.cpp

+
+                g_icon_manager->init(name_theme);
+
+                const QIcon create_user_icon = g_icon_manager->get_object_icon(OBJECT_CATEGORY_PERSON);


In my opinion better remove "create" word from icon object's name. Verbs are commonly used in function names

Samael340 · 2023-11-22T11:57:56Z

src/admc/main_window.cpp

+                g_icon_manager->init(name_theme);
+
+                const QIcon create_user_icon = g_icon_manager->get_object_icon(OBJECT_CATEGORY_PERSON);
+                ui->action_create_user->setIcon(create_user_icon);


To set action icons pass action pointers to icon manager someway. You can pass it when you initialize icon manager as an option

Samael340 · 2023-11-22T12:22:45Z

src/admc/settings.cpp

@@ -112,3 +112,17 @@ void settings_set_variant(const QString setting, const QVariant &value) {

    settings.setValue(setting, value);
 }
+
+QList<QString> settings_get_themes (){
+    QSettings::setPath(QSettings::NativeFormat, QSettings::SystemScope, "/etc/icon-theme-ActiveDirectory");


Are you sure this is a good idea to move configs to the system scope?

А что такое /etc/icon-theme-ActiveDirectory ?
Это файл настройки или каталог? Если каталог, то для чего?

Это каталог в котором лежит файл конфигураций с темами иконок.
Этот каталог и файл конфигураций в нем создаются при установке пакета с темами значков:
https://github.com/LenkaDEA/admc_icons/tree/themes

Samael340 · 2023-11-22T12:31:00Z

src/admc/settings.cpp

+
+    QStringList all_themes = set.allKeys();
+    for( QString &name_theme : all_themes )
+        name_theme_list.push_back(set.value(name_theme).toString());


I think better check themes availability. It was done somehow here #369

Здесь я читаю все темы из конфига. Если темы не установлены, то они и не будут доступны для выбора пользователя.
Что именно сделать как там?

7352f78#diff-97bc1ba0dc658854c9255d6c6a716e0f785f15abb59e7af85b2278a6038ade86R116

Если я верно поняла твою мысль, то ты хочешь, чтоб просматривался весь каталог с темами, тем самым в списке выбора тем появились доп темы, что есть у пользователя. Верно я тебя поняла?
Если да, то считаю, что это лишний функционал.

Нам не известно, как доп темы (которые установлены у пользователя на системе) будут дружить с нашим приложением.

Если пользователь захочет поменять тему, то он поменяет ее скорее на всей системе.

Наши темы более полные для нашего приложения, так что использовать, с большей вероятность, будут их, потому что это удобнее.

Если я не права, то поясните, потому это будет не лишним, пожалуйста.

Samael340 · 2023-11-22T12:44:52Z

.gear/admc.spec

@@ -28,6 +28,7 @@ BuildRequires: libkrb5-devel

 Requires: libsasl2
 Requires: libsasl2-plugin-gssapi
+Requires: icon-theme-ActiveDirectory


I suggest dividing theme changing feature and new themes package addition (main icon too) into two pull requests, but I am not sure about it.

…r icon themes

LenkaDEA force-pushed the icon-theme-ActiveDirectory branch from 6ef91ea to 066da7d Compare November 20, 2023 07:58

Samael340 requested changes Nov 22, 2023

View reviewed changes

LenkaDEA added 6 commits December 7, 2023 18:32

Added a selection of our three themes (line, duotone, color)

ae1c494

Changed the logic of displaying query icons to work correctly with ou…

cbc281f

…r icon themes

Added dependency to spec-file

023a61c

Added translation to select theme

e99046e

The application icon themes are taken from the config file

f4379b0

Added translation of icon theme names into Russian (from the config)

338f326

LenkaDEA force-pushed the icon-theme-ActiveDirectory branch from 066da7d to 338f326 Compare December 11, 2023 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Icon themes: Active-Directory (line, duotone, color) #425

Icon themes: Active-Directory (line, duotone, color) #425

LenkaDEA commented Nov 15, 2023

Samael340 Nov 22, 2023

Samael340 Nov 22, 2023

Samael340 Nov 22, 2023

Samael340 Nov 22, 2023

Samael340 Nov 22, 2023 •

edited

Loading

Samael340 Nov 22, 2023

Samael340 Nov 22, 2023

Samael340 Nov 22, 2023

mastersin Nov 23, 2023

LenkaDEA Nov 23, 2023

Samael340 Nov 22, 2023

LenkaDEA Nov 23, 2023

Samael340 Nov 23, 2023

LenkaDEA Nov 26, 2023

Samael340 Nov 22, 2023


		g_icon_manager->init(name_theme);

		const QIcon create_user_icon = g_icon_manager->get_object_icon(OBJECT_CATEGORY_PERSON);

Icon themes: Active-Directory (line, duotone, color) #425

Are you sure you want to change the base?

Icon themes: Active-Directory (line, duotone, color) #425

Conversation

LenkaDEA commented Nov 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Samael340 Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Samael340 Nov 22, 2023 •

edited

Loading