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 control panel search focus issue - SL #3188 #168

Closed
wants to merge 1 commit into from
Closed

Fix control panel search focus issue - SL #3188 #168

wants to merge 1 commit into from

Conversation

godiard
Copy link
Contributor

@godiard godiard commented Nov 28, 2013

Check if the window is active.

Fixes #3188

Signed-off-by: Gonzalo Odiard gonzalo@laptop.org

Check if the window is active.

Fixes #3188

Signed-off-by: Gonzalo Odiard <gonzalo@laptop.org>
@dnarvaez
Copy link
Contributor

I suspect the problem is that ControlPanel is a modal dialog, but not a proper one. A modal dialog should not allow you to activate another window in the same process, which seem to be the case here.

We are only calling show() on it, we should also block in an event loop. The easier way is to do that is to use GtkDialog run(), otherwise I think it's possible to do it for a GtkWindow too but it's a bit more complicated. Can we make ControlPanel a GtkDialog or is there a good reason for it to be GtkWindow?

@godiard
Copy link
Contributor Author

godiard commented Nov 29, 2013

Honestly, I don't know...

@dnarvaez
Copy link
Contributor

Looking at gtk_dialog_run it seems it might be enough to do something like this after the show()

loop = GObject.main_loop_new()
loop.run()

@godiard
Copy link
Contributor Author

godiard commented Dec 4, 2013

I have tried your proposal (attached patch) but show the same defect than
without the patch,
if the user click out of the window, every key pressed is show in the
search box,
but only the last letter.

Can you tell me if the attached patch is what you said or if I
misinterpreted your comment?

Is based in the code from Gtkdialog
https://git.gnome.org/browse/gtk+/tree/gtk/gtkdialog.c

On Fri, Nov 29, 2013 at 5:52 PM, Daniel Narvaez notifications@github.comwrote:

Looking at gtk_dialog_run it seems it might be enough to do something like
this after the show()

loop = GObject.main_loop_new()
loop.run()


Reply to this email directly or view it on GitHubhttps://github.com//pull/168#issuecomment-29537050
.

Gonzalo Odiard

@dnarvaez
Copy link
Contributor

dnarvaez commented Dec 4, 2013

No patch attached, I guess github gets rid of it.

@godiard
Copy link
Contributor Author

godiard commented Dec 4, 2013

Sending by traditional email :)

On Wed, Dec 4, 2013 at 4:10 PM, Daniel Narvaez notifications@github.comwrote:

No patch attached, I guess github gets rid of it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/168#issuecomment-29834886
.

Gonzalo Odiard

@godiard
Copy link
Contributor Author

godiard commented Dec 4, 2013

Yes I meant something like that.

I can't look deeper into it right now, but give me couple of days and I'll
get back to you.

On 4 December 2013 20:18, Gonzalo Odiard godiard@gmail.com wrote:

Sending by traditional email :)

On Wed, Dec 4, 2013 at 4:10 PM, Daniel Narvaez notifications@github.comwrote:

No patch attached, I guess github gets rid of it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/168#issuecomment-29834886
.

Gonzalo Odiard

Daniel Narvaez

@godiard
Copy link
Contributor Author

godiard commented Dec 4, 2013

A additional note. With this main_loop patch, I don't know if I did
something wrong,
but I can't close sugar

On Wed, Dec 4, 2013 at 4:20 PM, Daniel Narvaez dwnarvaez@gmail.com wrote:

Yes I meant something like that.

I can't look deeper into it right now, but give me couple of days and I'll
get back to you.

On 4 December 2013 20:18, Gonzalo Odiard godiard@gmail.com wrote:

Sending by traditional email :)

On Wed, Dec 4, 2013 at 4:10 PM, Daniel Narvaez notifications@github.comwrote:

No patch attached, I guess github gets rid of it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/168#issuecomment-29834886
.

Gonzalo Odiard

Daniel Narvaez

Gonzalo Odiard

@dnarvaez
Copy link
Contributor

dnarvaez commented Dec 7, 2013

I don't think this is related to modality and clicking outside the control panel at all. Try to just start the control panel and type something (without focusing search), you will get the same behavior. The issues are caused by this code, if you remove it, the UI behaves sanely

https://github.com/sugarlabs/sugar/blob/master/src/jarabe/controlpanel/gui.py#L205

That feels like an hack to me. I'm not sure if it can be made to work. I'd rather just focus the entry when opening the window/coming back from sections... Or something like that.

@dnarvaez dnarvaez closed this Dec 7, 2013
@dnarvaez
Copy link
Contributor

dnarvaez commented Dec 7, 2013

Uff ignore that comment, more debugging needed.

@dnarvaez dnarvaez reopened this Dec 7, 2013
@dnarvaez
Copy link
Contributor

dnarvaez commented Dec 7, 2013

Ok, I finally understood what is going. By default gtk selects the entry content on grab_focus. When we click out of the window, the modal dialog loses focus, though keyboad is grabbed and we get key press events. Thus every time a key is pressed we try to grab focus. That doesn't really affect focus, presumably because you can't grab focus inside an unfocused window, but it does cause the entry to be cleared. This is probably a gtk bug but it's quite a corner case.

Now, with that explained, I think your patch is a reasonable solution.

@dnarvaez
Copy link
Contributor

dnarvaez commented Dec 7, 2013

Pushed

@dnarvaez dnarvaez closed this Dec 7, 2013
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

2 participants