spyder-ide / qtawesome

Iconic fonts in PyQt and PySide applications
https://qtawesome.readthedocs.io/en/latest/index.html
MIT License
802 stars 105 forks source link

PR: Add PySide6 support for qta-browser #171

Closed kumattau closed 2 years ago

kumattau commented 2 years ago

This PR is for that qta-browser supports PySide6. I tested the PR on Linux(x11) and multi-screen.

Please note that qta-browser cannot show correct icons until PySide6's PYSIDE-1685 is released. By the following workaround, qta-browser works with current PySide6 6.2.0, but I does not apply the workaround because I think it is not responsibility of qta-browser.

diff --git a/qtawesome/iconic_font.py b/qtawesome/iconic_font.py
index a89a088..cc9caec 100644
--- a/qtawesome/iconic_font.py
+++ b/qtawesome/iconic_font.py
@@ -405,7 +405,7 @@ class IconicFont(QObject):

     def font(self, prefix, size):
         """Return a QFont corresponding to the given prefix and size."""
-        font = QFont(self.fontname[prefix])
+        font = QFont([self.fontname[prefix]])
         font.setPixelSize(round(size))
         if prefix[-1] == 's':  # solid style
             font.setStyleName('Solid')

Fixes #170

5yutan5 commented 2 years ago

Hi. How about using setFamily() instead of the qfont constructor? Maybe it can be used with both qt5 and qt6.

diff --git a/qtawesome/iconic_font.py b/qtawesome/iconic_font.py
index a89a088..ca3ca1b 100644
--- a/qtawesome/iconic_font.py
+++ b/qtawesome/iconic_font.py
@@ -405,7 +405,8 @@ class IconicFont(QObject):

     def font(self, prefix, size):
         """Return a QFont corresponding to the given prefix and size."""
-        font = QFont(self.fontname[prefix])
+        font = QFont()
+        font.setFamily(self.fontname[prefix])
         font.setPixelSize(round(size))
         if prefix[-1] == 's':  # solid style
             font.setStyleName('Solid')
kumattau commented 2 years ago

@5yutan5, Thank you for the comment.

How about using setFamily() instead of the qfont constructor?

I checked it worked well on pyqt5 and pyside6. I added the commit.

5yutan5 commented 2 years ago

@kumattau, No problem. Thanks for this work👍

5yutan5 commented 2 years ago

About QApplication.desktop() problem, if you use QGuiApplication.primaryScreen() does not require exception handling. But I don't understand the difference between QGuiApplication.primaryScreen() and QGuiApplication.screenAt(). Perhaps this suggestion isn't really needed.

diff --git a/qtawesome/icon_browser.py b/qtawesome/icon_browser.py
index ea11916..7eb0e63 100644
--- a/qtawesome/icon_browser.py
+++ b/qtawesome/icon_browser.py
@@ -95,14 +95,8 @@ class IconBrowser(QtWidgets.QMainWindow):

         # QApplication.desktop() has been removed in Qt 6.
         # Instead, QGuiApplication.screenAt is supported in Qt 5.10 or later.
-        try:
-            desktop = QtWidgets.QApplication.desktop()
-            screen = desktop.screenNumber(desktop.cursor().pos())
-            centerPoint = desktop.screenGeometry(screen).center()
-        except:
-            screen = QtGui.QGuiApplication.screenAt(QtGui.QCursor.pos())
-            centerPoint = screen.geometry().center()
-
+        geo = self.geometry()
+        centerPoint = QtGui.QGuiApplication.primaryScreen().availableGeometry().center()
         geo.moveCenter(centerPoint)
         self.setGeometry(geo)
kumattau commented 2 years ago

Hi, @5yutan5

But I don't understand the difference between QGuiApplication.primaryScreen() and QGuiApplication.screenAt().

There is the following difference if screen#0 (primary) and screen#1 (secondary) exist.

The original code and my fix show a window on the screen where there is a cursor, but your suggestion always shows a window on the primary screen regardless of a cursor position.

5yutan5 commented 2 years ago

@kumattau, ok. Is this mean that using QGuiApplication.primaryScreen() in multiscreen may open the window in unexpected size or screen? I test only one screen so I couldn't find the difference😅

kumattau commented 2 years ago

Is this mean that using QGuiApplication.primaryScreen() in multiscreen may open the window in unexpected size or screen?

@5yutan5, yes that's right.

kumattau commented 2 years ago

@dalthviz thank you for the review. I applied all your suggestions and updated comment.