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

Improvements #31

Merged
merged 7 commits into from
May 8, 2017
Merged

Improvements #31

merged 7 commits into from
May 8, 2017

Conversation

allestuetsmerweh
Copy link
Owner

  • Move to Python 3
  • Adhere to PEP8
  • Update to new cefpython API
  • Drop obsolete code

@allestuetsmerweh allestuetsmerweh self-assigned this Apr 27, 2017
@allestuetsmerweh allestuetsmerweh requested a review from jegger April 27, 2017 22:55
windowInfo.SetAsOffscreen(wid)
Logger.debug("Use window handle %s, because: %s", window_id, e)
window_info = cefpython.WindowInfo()
window_info.SetAsOffscreen(window_id)

Choose a reason for hiding this comment

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

There have been modifications in cefpython's Print functionality. Printing should now work even when parent window handle is not provided in SetAsOffscreen. However I don't remember if window handle was also required for other reasons, whether it is required for mouse context menu and popup widgets to work correctly.

message_text,
default_prompt_text,
callback,
suppress_message,

Choose a reason for hiding this comment

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

In case you haven't seen, you can now remove unused args from callbacks signature and use **kwargs, see: https://github.com/cztomczak/cefpython/blob/master/docs/Migration-guide.md#v553-handlers-callbacks-and-other-interfaces

def OnAddressChange(self, browser, frame, url):
if browser.GetMainFrame()==frame:
def OnAddressChange(self, browser, frame, url): # noqa: N802
if browser.GetMainFrame() == frame:

Choose a reason for hiding this comment

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

This could be written: frame.IsMain().

Also do not compare browser/frame objects using == operator, because there are some edge cases when this doesn't work. Always use GetIdentifier() on these objects.

@allestuetsmerweh
Copy link
Owner Author

Thanks, @cztomczak . I'm not available over the prolonged weekend, so I'll look into adjustments afterwards also awaiting @jegger 's review.

jegger added 2 commits May 8, 2017 11:12
…) (to not spam the log by default); Disable context-menu as the diaplyed options are not something we want (less is more, end users do not need this functionality in our apps).
@@ -2,4 +2,4 @@
# 1) we don't load dependencies by storing it in __init__.py
# 2) we can import it in setup.py for the same reason
# 3) we can import it into your module module
__version__ = '0.5.57'
Copy link
Owner Author

@allestuetsmerweh allestuetsmerweh May 8, 2017

Choose a reason for hiding this comment

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

@jegger you want to change that together with the rest of the PR? Or did you just want to push this to master?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Together with the rest of this PR.

@jegger
Copy link
Collaborator

jegger commented May 8, 2017

We need a way to suppress the gtk file dialog at file upload. The dialog must have been introduced in one of the newer cefpython versions. The rest seems to be running fine (did not test it with Py3 tough).

@allestuetsmerweh
Copy link
Owner Author

allestuetsmerweh commented May 8, 2017

http://magpcss.org/ceforum/apidocs3/projects/(default)/CefDialogHandler.html

@cztomczak OnFileDialog is not implemented in cefpython, right?

…n capital. However I do not know if removing this breaks existing functionality. => Ctrl+a did not work before either...
@@ -192,11 +192,6 @@ def get_windows_key_code(self, kivycode):
# OnPreKeyEvent info for key events and replacate it here.
# (key codes can also be found on MSDN Virtual-key codes page)

# Must map basic letters, otherwise eg. ctrl+a to select all
Copy link
Collaborator

Choose a reason for hiding this comment

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

@cztomczak Do you remember what this exactly does?

Choose a reason for hiding this comment

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

ctrl+a selects all text on a web page - check if it works after that code was removed

Choose a reason for hiding this comment

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

Depending on context ctrl+a may select all text in a form

Copy link
Collaborator

Choose a reason for hiding this comment

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

Did not work before (and does not work with this bit of code removed). Maybe it was working some time ago. We never used this feature - so we never tested it - so I sadly have no idea when (I) broke that. But thanks for the clarification.

Choose a reason for hiding this comment

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

Possibly you removed some other code and it broke. It works in kivy_.py example in cefpython.

@cztomczak
Copy link

File dialog default implementation added here:
cztomczak/cefpython#241 (comment)

You can create a new issue to expose DialogHandler and its OnFileDialog callback, to be able to suppress it.

@allestuetsmerweh allestuetsmerweh merged commit c028b82 into master May 8, 2017
@allestuetsmerweh allestuetsmerweh deleted the update_py3_pep8_api branch May 8, 2017 15:35
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.

3 participants