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

WebUI: Add missing icons #21162

Merged
merged 4 commits into from
Aug 17, 2024
Merged

WebUI: Add missing icons #21162

merged 4 commits into from
Aug 17, 2024

Conversation

skomerko
Copy link
Contributor

@skomerko skomerko commented Aug 7, 2024

This PR adds missing icons to WebUI (in tabs, buttons, etc.).


To achieve better icon contrast on hover/selection (+ to change color for some icons in dark mode) I decided to use filter property. It is not ideal but right now it's probably the best way to do this with minimal amount of changes. There are many different SVG techniques but for now I kept everything in the same style and it just works.

image

@@ -1563,6 +1563,7 @@ window.addEventListener("DOMContentLoaded", () => {
const id = "uploadPage";
new MochaUI.Window({
id: id,
icon: "images/qbittorrent-tray.svg",
Copy link
Contributor Author

@skomerko skomerko Aug 7, 2024

Choose a reason for hiding this comment

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

I added it like this to all mocha windows but down the line common properties / callbacks should probably be contained in some base object and then spread during window creation.

@Chocobo1 Chocobo1 added the WebUI WebUI-related issues/changes label Aug 7, 2024
@Chocobo1 Chocobo1 requested a review from a team August 9, 2024 07:00
@Chocobo1 Chocobo1 added this to the 5.1 milestone Aug 9, 2024
thalieht
thalieht previously approved these changes Aug 9, 2024
Chocobo1
Chocobo1 previously approved these changes Aug 10, 2024
@Chocobo1
Copy link
Member

@skomerko
This has merge conflicts.

@Chocobo1 Chocobo1 merged commit e069fbc into qbittorrent:master Aug 17, 2024
14 checks passed
@Chocobo1
Copy link
Member

@skomerko
Thank you!

@@ -52,6 +52,11 @@ tr.dynamicTableHeader {
white-space: nowrap;
}

.dynamicTable tr.selected img,
.dynamicTable tr:hover img {
filter: var(--color-icon-hover);
Copy link
Contributor

@HanabishiRecca HanabishiRecca Aug 19, 2024

Choose a reason for hiding this comment

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

This change breaks peer country flags on selected/hover lines.

screenshot
screenshot

Copy link
Contributor Author

@skomerko skomerko Aug 19, 2024

Choose a reason for hiding this comment

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

Will fix it, thanks for catching it.

@@ -540,23 +540,29 @@ ul.filterList {
padding-left: 0;
}

ul.filterList li:hover img,
ul.filterList .selectedFilter img {
filter: var(--color-icon-hover);
Copy link
Contributor

Choose a reason for hiding this comment

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

This removes color from selected icons. Is it intentional? I don't really like that to be honest.

screenshot
screenshot

Copy link
Contributor Author

@skomerko skomerko Aug 19, 2024

Choose a reason for hiding this comment

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

Yes, otherwise it would look like this:
image
At least now they are visible.

How would you like it to be?

Copy link
Contributor

@HanabishiRecca HanabishiRecca Aug 19, 2024

Choose a reason for hiding this comment

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

Maybe use other filter? Like

Suggested change
filter: var(--color-icon-hover);
filter: drop-shadow(0 0 1px #000);

screenshot

Or more tricky

Suggested change
filter: var(--color-icon-hover);
filter: drop-shadow(0 1px 0 #000) drop-shadow(0 -1px 0 #000) drop-shadow(1px 0 0 #000) drop-shadow(-1px 0 0 #000);

screenshot

if we want it to be solid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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