-
-
Notifications
You must be signed in to change notification settings - Fork 68
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
Settings applications tab #412
base: main
Are you sure you want to change the base?
Conversation
The existing patch failed, most probably during migration to PyQt6 Switch from `.ident` property to `.whatsThis()` and `.setWhatsThis()`
Since PyQt6.QtGui.QImage does not provide tinting and the available effects from `QPainter` does not produce the same result, a helper function is necessary.
3122c48
to
ebaf466
Compare
This likely needs --enablerepo flag like here: QubesOS/qubes-core-admin-client@c3759ad |
ebaf466
to
a6a0b80
Compare
Did not work p.s. Trying to fetch it from source. |
a8fc947
to
7b3e15d
Compare
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025031004-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025021804-4.3&flavor=update
Failed tests32 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/129058#dependencies 8 fixed
Unstable testsPerformance TestsPerformance degradation:28 performance degradations
Remaining performance tests:44 tests
|
7b3e15d
to
64d89d3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #412 +/- ##
==========================================
+ Coverage 68.33% 68.50% +0.16%
==========================================
Files 17 17
Lines 3708 3762 +54
==========================================
+ Hits 2534 2577 +43
- Misses 1174 1185 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
OK. Unittests & Pylint are finally green (after hamnering my way to installing qubesimgconverter). The OpenQA installedTest fails most probably because of the icons. |
Yes, all of those are due to different appearance of the applications tab. If Marta says she likes it, I'll adjust base screenshots ("needles"), unfortunately there is no easy way to do that for external contributors. |
template_icons_path = path.join( | ||
path.expanduser("~"), | ||
".local", | ||
"share", | ||
"qubes-appmenus", | ||
f"{main_template}", | ||
"apps.tempicons", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better to extend qvm-appmenus
to print the path too? Maybe add some "field" like OriginalIconPath
?
Especially, the current implementation doesn't handle applications installed in the AppVM itself (you can put applications into ~/.local/share/applications
and they will be available same as those in /usr/share/applications
in the template).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better to extend
qvm-appmenus
to print the path too? Maybe add some "field" likeOriginalIconPath
? Especially, the current implementation doesn't handle applications installed in the AppVM itself (you can put applications into~/.local/share/applications
and they will be available same as those in/usr/share/applications
in the template).
Yes. It does not work with icons for flatpak, snap and similar cases (will show the VM's label icon instead). But the fix for qvm-appmenus
will be for desktop-linux-common
repository. I suggest we leave this for a supplementary PR. After qvm-appemnus
is patched to provide OriginalIconPath
, I will come back and fix it here as well. This PR is already long and complex and I want to work on the pending Qube Comment PRs as well (after some of the pending PRs are concluded).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P.S. One more consideration. The GUIVM icons in ~/.local/share/qubes-appmenus/VMNAME/apps.icons
provided via qvm-appmenus are already tinted (applicable for Qube specific Flatpak, Snap, ...). And we have to consider this as well if we ever want to patch qvm-appmenus
to provide real original icon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GUIVM icons in
~/.local/share/qubes-appmenus/VMNAME/apps.icons
Yes, but there is also apps.tempicons
dir, similar to icons from the template.
The current patch adds regular tinted icons. Option for fade in/out to regular images (not tinted) icons on mouse hover is on TODO list. fixes: QubesOS/qubes-issues#9829
64d89d3
to
52f6879
Compare
This PR contains 3 individual commits:
fixes: QubesOS/qubes-issues#9829