6
\$\begingroup\$

I wanted to contribute to the PyFladesk project. The objective is to have a function that receives a Flask's app and embeds it in a Qt desktop app.

I've already create a Pull Request on the main repository, individual commmits and changes can be viewed there

This was the initial version: 83 lines - 3 Classes - 1 Function - Uses PyQt4

import sys,webbrowser

from PyQt4.QtCore import QThread, QUrl,SIGNAL,QSize
from PyQt4.QtGui import QApplication,QMainWindow,QIcon
from PyQt4.QtWebKit import QWebView,QWebPage

# CONFIG
PORT = 5000
ROOT_URL = 'http://localhost:{}'.format(PORT)
WIDTH = 300
HEIGHT = 400
WINDOW_TITLE = "PyFladesk"
ICON = 'appicon.png'


# run flask on seperate theared
class FlaskThread(QThread):
    def __init__(self, application):
        QThread.__init__(self)
        self.application = application

    def __del__(self):
        self.wait()

    def run(self):
        self.application.run(port=PORT)


class MainWindow(QMainWindow):

    def __init__(self):
        QMainWindow.__init__(self)
        self.resize (WIDTH , HEIGHT)
        self.setWindowTitle(WINDOW_TITLE)
        self.webView = WebView(self)
        self.setCentralWidget(self.webView)



class WebView(QWebView):
    def __init__(self ,parent=None):
        super(WebView,self).__init__(parent)

    def dragEnterEvent(self,e):
        e.ignore()

    def dropEvent(self,e):
        e.ignore()

    def contextMenuEvent(self,e):
        pass

    # open links in default browser 
    # stolen from http://stackoverflow.com/a/3188942/1103397 :D
    def linkClicked(self,url): 
        webbrowser.open(url.toEncoded().data())


def provide_GUI_for(application):
    qtapp = QApplication(sys.argv)

    webapp = FlaskThread(application)
    webapp.start()

    qtapp.aboutToQuit.connect(webapp.terminate)

    mainWindow = MainWindow()
    # set app icon    
    mainWindow.setWindowIcon(QIcon(ICON))

    # prevent open urls in QWebView.
    mainWindow.webView.page().setLinkDelegationPolicy(QWebPage.DelegateAllLinks)
    mainWindow.webView.connect(mainWindow.webView, SIGNAL("linkClicked (const QUrl&)"), mainWindow.webView.linkClicked)

    mainWindow.webView.load(QUrl(ROOT_URL))
    mainWindow.show()

    return qtapp.exec_()


if __name__ == '__main__':
    from routes import app
    provide_GUI_for(app)

And this is the updated version: 50 lines - 1 Function - Uses PyQt 5.10.0

import sys
from PyQt5 import QtCore, QtWidgets, QtGui, QtWebEngineWidgets


def init_gui(application, port=5000, width=300, height=400,
            window_title="PyFladesk", icon="appicon.png"):

    ROOT_URL = 'http://localhost:{}'.format(port)

    # open links in browser from http://stackoverflow.com/a/3188942/1103397 :D
    # thanks to https://github.com/marczellm/qhangups/blob/cfed73ee4383caed1568c0183a9906180f01cb00/qhangups/WebEnginePage.py
    def link_clicked(url, typ, ismainframe):
        ready_url = url.toEncoded().data().decode()
        is_clicked = typ == QtWebEngineWidgets.QWebEnginePage.NavigationTypeLinkClicked
        is_not_internal = ROOT_URL not in ready_url
        if is_clicked and is_not_internal:
            QtGui.QDesktopServices.openUrl(url)
            return False
        return True

    def run_app():
        application.run(port=port, threaded=True)

    # Application Level
    qtapp = QtWidgets.QApplication(sys.argv)
    webapp = QtCore.QThread()
    webapp.__del__ = webapp.wait
    webapp.run = run_app
    webapp.start()
    qtapp.aboutToQuit.connect(webapp.terminate)

    # Main Window Level
    window = QtWidgets.QMainWindow()
    window.resize(width, height)
    window.setWindowTitle(window_title)
    window.setWindowIcon(QtGui.QIcon(icon))

    # WebView Level
    window.webView = QtWebEngineWidgets.QWebEngineView(window)
    window.setCentralWidget(window.webView)

    # WebPage Level
    page = QtWebEngineWidgets.QWebEnginePage()
    page.acceptNavigationRequest = link_clicked
    page.load(QtCore.QUrl(ROOT_URL))
    window.webView.setPage(page)

    window.show()

    return qtapp.exec_()

Some of my concerns are:

  • Is it a good decision not to use classes at all?

  • Is the code readable and maintainable?

  • Are there any suggestions to improve it?

This was once review here

\$\endgroup\$
2
\$\begingroup\$

I’m not too found of putting it all into a single method as:

  1. it seems to have too much responsibilities;
  2. building some objects (QThread, QWebEnginePage) is really awkward when methods are reassigned.

I really like using default values for parameters instead of global constants, though.

Instead I’d go with an intermediate state where I’d use proper subclassing instead of methods assignment:

import sys
from PyQt5 import QtCore, QtWidgets, QtGui, QtWebEngineWidgets


class ApplicationThread(QtCore.QThread):
    def __init__(self, application, port=5000):
        super(ApplicationThread, self).__init__()
        self.application = application
        self.port = port

    def __del__(self):
        self.wait()

    def run(self):
        self.application.run(port=self.port, threaded=True)


class WebPage(QtWebEngineWidgets.QWebEnginePage):
    def __init__(self, root_url):
        super(WebPage, self).__init__()
        self.root_url = root_url

    def home(self):
        self.load(QtCore.QUrl(self.root_url))

    def acceptNavigationRequest(self, url, kind, is_main_frame):
        """open external links in browser and internal links in the regular webview"""
        # thanks to https://github.com/marczellm/qhangups/blob/cfed73ee4383caed1568c0183a9906180f01cb00/qhangups/WebEnginePage.py
        ready_url = url.toEncoded().data().decode()
        is_clicked = kind == self.NavigationTypeLinkClicked
        if is_clicked and self.root_url not in ready_url:
            QtGui.QDesktopServices.openUrl(url)
            return False
        return super(WebPage, self).acceptNavigationRequest(url, kind, is_main_frame)


def init_gui(application, port=5000, width=300, height=400,
             window_title="PyFladesk", icon="appicon.png", argv=None):
    if argv is None:
        argv = sys.argv

    # Application Level
    qtapp = QtWidgets.QApplication(argv)
    webapp = ApplicationThread(application, port)
    webapp.start()
    qtapp.aboutToQuit.connect(webapp.terminate)

    # Main Window Level
    window = QtWidgets.QMainWindow()
    window.resize(width, height)
    window.setWindowTitle(window_title)
    window.setWindowIcon(QtGui.QIcon(icon))

    # WebView Level
    webView = QtWebEngineWidgets.QWebEngineView(window)
    window.setCentralWidget(webView)

    # WebPage Level
    page = WebPage('http://localhost:{}'.format(port))
    page.home()
    webView.setPage(page)

    window.show()
    return qtapp.exec_()

Back to two classes and a few more lines, but subclassing really feels better here. I also removed assigning the web-view as a window attribute as I don't see the need.

Lastly, I added support for user-provided argv (for testing purposes), just in case; not sure if really needed.

\$\endgroup\$
  • \$\begingroup\$ My question is, is it worth it to declare a class just to redefine 3 methods, all of them magic methods (ApplicationThread)? \$\endgroup\$ – Ezequiel Castaño Feb 22 '18 at 22:57
  • \$\begingroup\$ I look at it the other way around. Is it worth it to cram everything into a god method only for the sake of removing classes? To me, the ugly part is how you reassign methods; when subclassing and overriding are way more natural (and I didn't have to invent anything, just reuse the functions you were already using). \$\endgroup\$ – Mathias Ettinger Feb 22 '18 at 23:22
  • \$\begingroup\$ You are right, maybe I'm trying too hard to avoid OOP in Python but it seems like it's the Qt style to subclass and redefine methods. \$\endgroup\$ – Ezequiel Castaño Feb 23 '18 at 0:05
  • \$\begingroup\$ Don't get me wrong, there are times when I think OOP is the wrong tool for the job. But yes, this is Qt, everything is classes already. \$\endgroup\$ – Mathias Ettinger Feb 23 '18 at 8:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.