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

Adding QWebEngineView #273

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Adding QWebEngineView #273

wants to merge 1 commit into from

Conversation

krets
Copy link

@krets krets commented Jan 30, 2018

This gives me enough to port my existing QWebView widgets into QWebEngineView.

@CLAassistant
Copy link

CLAassistant commented Jan 30, 2018

CLA assistant check
All committers have signed the CLA.

@krets
Copy link
Author

krets commented Jan 30, 2018

I committed this with the wrong author email. I'll update and re-pull-request.

@krets krets closed this Jan 30, 2018
@krets
Copy link
Author

krets commented Jan 30, 2018

It looks like github managed my --amend.

edit:
Since I can't comment, I'll sneak in my last words here.

It looks like Travis is failing because I blindly setup the remap from QtWebKitWidgets into QtWebEngineWidgets. I don't fully understand the design of the tool, so I have probably added the QtWebEngineWidgets in the wrong spot.

I'll see if I can get this working with QtSiteConfig. Thanks for the suggestion.

@krets krets reopened this Jan 30, 2018
@fredrikaverpil
Copy link
Collaborator

fredrikaverpil commented Jan 30, 2018

Those modules won't be available on all bindings, in accordance with the VFX Platform specs, as you can see from the failing tests.

I would recommend you look into QtSiteConfig.py to add these on your end.

@mottosso
Copy link
Owner

Continuing from the Maya Python mailing list.

@krets Thanks for the pull request. I haven't used these web widgets myself, but my impression is that they are too different to be treated as one and the same; one wrapping WebKit and the other Chromium.

For completeness, the members exposed through Qt.py are those that are identical across all bindings, such that you can write an application in any binding and know for sure it will run without problems in all other bindings without changes to the original application.

If you are confident that QWebView qualifies, then I see no problem including it in Qt.py. What I'd need is tests that exercise the most important functionality of QWebView. Functionality that isn't portable should be made an example of as Caveats.

Let me know if you have any questions!

@fredrikaverpil
Copy link
Collaborator

fredrikaverpil commented Jan 31, 2018

@krets in case QtC decided to move members around between Qt4 and Qt5 in regards to QtWebEngineWidgets, you need to figure out (seems like you already did) how these were moved/renamed and then we'll have to include this here: https://github.com/mottosso/Qt.py/blob/master/membership.py#L62

The build_membership.sh script rolls through all members of PySide, PyQt4, PySide2 and PyQt5 and spits out a JSON file for each one. Then, the JSON files are compared and a single common_members.json is produced which will only contain members which exist in all the four main JSON files. But if members were moved between Qt4-Qt5, you'll have to address this in the copy_qtgui_to_modules() function.

So the idea is that we should copy the common_members.json contents into Qt.py's common_members.json. This way, we can quickly identify new members whenever e.g. PySide2 is developed further and supports new members.

So in this case, you should make sure that you can see the following in the generated common_members.json file (by editing copy_qtgui_to_modules):

    "QtWebEngineWidgets": [
        "QWebEnginePage",
        "QWebEngineView",
        "QWebEngineSettings"
    ],

Feel free to rename the copy_qtgui_to_modules function itself, as it doesn't only copy QtGui any longer.

@fredrikaverpil
Copy link
Collaborator

Edited my previous post for clarity..

@dgovil dgovil mentioned this pull request Mar 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants