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

Fix searching text in our Web widgets #3266

Merged
merged 4 commits into from
Jun 28, 2016
Merged

Conversation

ccordoba12
Copy link
Member

Fixes #3159

@ccordoba12 ccordoba12 added this to the v3.0beta4 milestone Jun 27, 2016
@ccordoba12 ccordoba12 changed the title Fix searching text in Web widgets Fix searching text in our Web widgets Jun 27, 2016
@@ -185,7 +185,7 @@ def __init__(self, parent):

self.webview = FrameWebView(self)
self.find_widget = FindReplace(self)
self.find_widget.set_editor(self.webview)
self.find_widget.set_editor(self.webview._webview)
Copy link
Contributor

Choose a reason for hiding this comment

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

Using a private variable is not good practice. Maybe just add a property to self.webview to access _webview ? This way no other code has to change.

Copy link
Member Author

@ccordoba12 ccordoba12 Jun 28, 2016

Choose a reason for hiding this comment

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

This was a change made by @goanpeca, so the real webview widget is saved in a private variable.

I'll write a new method to access it :-)

Copy link
Member

Choose a reason for hiding this comment

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

Yes I remember now!, but @Nodd is ok, just add a property :-p

@Nodd
Copy link
Contributor

Nodd commented Jun 28, 2016

I made a few comments, mainly to simplify the code.

@ccordoba12 ccordoba12 merged commit 5052a28 into spyder-ide:master Jun 28, 2016
@ccordoba12 ccordoba12 deleted the i3159 branch June 28, 2016 19:31
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