The following patch fixes the bug #975838 by moving the focus changes handling from the MountpointSelector widget to the Page and spoke it appears in, respectively. The controversy lies in making the MountpointSelector's behaviour relying on the widget it is placed into. The MounpointSelector widget per se no longer handles focus changes correctly and it looks like being selected/chosen even if it loses focus. However, this is what we want -- to keep it look like that when user clicks to the right part of the screen to change parameters of the mount point being chosen. And since I doubt anybody will reuse the MountpointSelector as a standalone widget without the Page and the Accordion I think this change could be done.
I've recorded a video preview [1] showing the functionality with this patch applied.
[1] http://vpodzime.fedorapeople.org/MountpointSelector_focus.webm
Let me know if I should add more comments to some places.
Vratislav Podzimek (1): Handle focus changes of MountpointSelectors from outside (#975838)
pyanaconda/ui/gui/spokes/custom.py | 2 ++ pyanaconda/ui/gui/spokes/lib/accordion.py | 6 ++++++ widgets/src/MountpointSelector.c | 31 ++++++++----------------------- 3 files changed, 16 insertions(+), 23 deletions(-)
We handle clicking and pressing keys on the MountpointSelectors from outside which allows us to do some magic the selectors have no clue about. Doing the same with focus changes (that happen e.g. when using arrows) allows us to keep the MountpointSelector selected (with blue background and the right arrow shown) even if the focus moves to some other part of the window.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- pyanaconda/ui/gui/spokes/custom.py | 2 ++ pyanaconda/ui/gui/spokes/lib/accordion.py | 6 ++++++ widgets/src/MountpointSelector.c | 31 ++++++++----------------------- 3 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index 8a2b9cd..3297359 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -2405,6 +2405,8 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): # Take care of the previously chosen selector. if self._current_selector and self._initialized and \ self._current_selector != selector: + # unselect the previously chosen selector + self._current_selector.set_chosen(False) self._save_current_selector() log.debug("new selector: %s" % selector._device)
diff --git a/pyanaconda/ui/gui/spokes/lib/accordion.py b/pyanaconda/ui/gui/spokes/lib/accordion.py index 3531e89..6310003 100644 --- a/pyanaconda/ui/gui/spokes/lib/accordion.py +++ b/pyanaconda/ui/gui/spokes/lib/accordion.py @@ -176,6 +176,7 @@ class Page(Gtk.Box): selector = selectorFromDevice(device, mountpoint=mountpoint) selector.connect("button-press-event", self._onSelectorClicked, cb) selector.connect("key-release-event", self._onSelectorClicked, cb) + selector.connect("focus-in-event", self._onSelectorFocusIn, cb) selector.set_margin_bottom(6) self.members.append(selector)
@@ -218,6 +219,11 @@ class Page(Gtk.Box): # show the details for the newly selected object. cb(selector)
+ def _onSelectorFocusIn(self, selector, event, cb): + # could be simple lambda, but this way it looks more similar to the + # _onSelectorClicked + cb(selector) + class UnknownPage(Page): def __init__(self, title): # For this type of page, there's only one place to store members. diff --git a/widgets/src/MountpointSelector.c b/widgets/src/MountpointSelector.c index 726b42e..c1e4b87 100644 --- a/widgets/src/MountpointSelector.c +++ b/widgets/src/MountpointSelector.c @@ -67,8 +67,7 @@ static void anaconda_mountpoint_selector_set_property(GObject *object, guint pro static void anaconda_mountpoint_selector_realize(GtkWidget *widget, gpointer user_data); static void anaconda_mountpoint_selector_finalize(AnacondaMountpointSelector *widget);
-static void anaconda_mountpoint_selector_toggle_background(AnacondaMountpointSelector *widget); -static gboolean anaconda_mountpoint_selector_focus_changed(GtkWidget *widget, GdkEventFocus *event, gpointer user_data); +static void anaconda_mountpoint_selector_toggle_background(AnacondaMountpointSelector *widget);
static void anaconda_mountpoint_selector_class_init(AnacondaMountpointSelectorClass *klass) { GObjectClass *object_class = G_OBJECT_CLASS(klass); @@ -176,8 +175,6 @@ static void anaconda_mountpoint_selector_init(AnacondaMountpointSelector *mountp */ gtk_widget_set_can_focus(GTK_WIDGET(mountpoint), TRUE); gtk_widget_add_events(GTK_WIDGET(mountpoint), GDK_FOCUS_CHANGE_MASK|GDK_KEY_RELEASE_MASK); - g_signal_connect(mountpoint, "focus-in-event", G_CALLBACK(anaconda_mountpoint_selector_focus_changed), NULL); - g_signal_connect(mountpoint, "focus-out-event", G_CALLBACK(anaconda_mountpoint_selector_focus_changed), NULL);
/* Set "hand" cursor shape when over the selector */ mountpoint->priv->cursor = gdk_cursor_new(GDK_HAND2); @@ -275,24 +272,6 @@ static void anaconda_mountpoint_selector_set_property(GObject *object, guint pro } }
-static gboolean anaconda_mountpoint_selector_focus_changed(GtkWidget *widget, GdkEventFocus *event, gpointer user_data) { - GtkStateFlags new_state; - - new_state = gtk_widget_get_state_flags(widget) & ~GTK_STATE_FOCUSED; - if (event->in) { - gtk_widget_show(GTK_WIDGET(ANACONDA_MOUNTPOINT_SELECTOR(widget)->priv->arrow)); - new_state |= GTK_STATE_FOCUSED; - anaconda_mountpoint_selector_set_chosen(ANACONDA_MOUNTPOINT_SELECTOR(widget), TRUE); - } - else { - gtk_widget_hide(GTK_WIDGET(ANACONDA_MOUNTPOINT_SELECTOR(widget)->priv->arrow)); - anaconda_mountpoint_selector_set_chosen(ANACONDA_MOUNTPOINT_SELECTOR(widget), FALSE); - } - - gtk_widget_set_state_flags(widget, new_state, TRUE); - return FALSE; -} - static void anaconda_mountpoint_selector_toggle_background(AnacondaMountpointSelector *widget) { if (widget->priv->chosen) { gtk_widget_set_state_flags(GTK_WIDGET(widget), GTK_STATE_FLAG_SELECTED, FALSE); @@ -333,6 +312,12 @@ gboolean anaconda_mountpoint_selector_get_chosen(AnacondaMountpointSelector *wid void anaconda_mountpoint_selector_set_chosen(AnacondaMountpointSelector *widget, gboolean is_chosen) { widget->priv->chosen = is_chosen; anaconda_mountpoint_selector_toggle_background(widget); - if (is_chosen) + + if (is_chosen) { + gtk_widget_show(GTK_WIDGET(widget->priv->arrow)); gtk_widget_grab_focus(GTK_WIDGET(widget)); + } + else { + gtk_widget_hide(GTK_WIDGET(widget->priv->arrow)); + } }
On Mon, 2013-07-29 at 14:39 +0200, Vratislav Podzimek wrote:
We handle clicking and pressing keys on the MountpointSelectors from outside which allows us to do some magic the selectors have no clue about. Doing the same with focus changes (that happen e.g. when using arrows) allows us to keep the MountpointSelector selected (with blue background and the right arrow shown) even if the focus moves to some other part of the window.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com
pyanaconda/ui/gui/spokes/custom.py | 2 ++ pyanaconda/ui/gui/spokes/lib/accordion.py | 6 ++++++ widgets/src/MountpointSelector.c | 31 ++++++++----------------------- 3 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index 8a2b9cd..3297359 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -2405,6 +2405,8 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): # Take care of the previously chosen selector. if self._current_selector and self._initialized and \ self._current_selector != selector:
# unselect the previously chosen selector
self._current_selector.set_chosen(False) self._save_current_selector() log.debug("new selector: %s" % selector._device)
diff --git a/pyanaconda/ui/gui/spokes/lib/accordion.py b/pyanaconda/ui/gui/spokes/lib/accordion.py index 3531e89..6310003 100644 --- a/pyanaconda/ui/gui/spokes/lib/accordion.py +++ b/pyanaconda/ui/gui/spokes/lib/accordion.py @@ -176,6 +176,7 @@ class Page(Gtk.Box): selector = selectorFromDevice(device, mountpoint=mountpoint) selector.connect("button-press-event", self._onSelectorClicked, cb) selector.connect("key-release-event", self._onSelectorClicked, cb)
selector.connect("focus-in-event", self._onSelectorFocusIn, cb) selector.set_margin_bottom(6) self.members.append(selector)
@@ -218,6 +219,11 @@ class Page(Gtk.Box): # show the details for the newly selected object. cb(selector)
- def _onSelectorFocusIn(self, selector, event, cb):
# could be simple lambda, but this way it looks more similar to the
# _onSelectorClicked
cb(selector)
class UnknownPage(Page): def __init__(self, title): # For this type of page, there's only one place to store members. diff --git a/widgets/src/MountpointSelector.c b/widgets/src/MountpointSelector.c index 726b42e..c1e4b87 100644 --- a/widgets/src/MountpointSelector.c +++ b/widgets/src/MountpointSelector.c @@ -67,8 +67,7 @@ static void anaconda_mountpoint_selector_set_property(GObject *object, guint pro static void anaconda_mountpoint_selector_realize(GtkWidget *widget, gpointer user_data); static void anaconda_mountpoint_selector_finalize(AnacondaMountpointSelector *widget);
-static void anaconda_mountpoint_selector_toggle_background(AnacondaMountpointSelector *widget); -static gboolean anaconda_mountpoint_selector_focus_changed(GtkWidget *widget, GdkEventFocus *event, gpointer user_data); +static void anaconda_mountpoint_selector_toggle_background(AnacondaMountpointSelector *widget);
static void anaconda_mountpoint_selector_class_init(AnacondaMountpointSelectorClass *klass) { GObjectClass *object_class = G_OBJECT_CLASS(klass); @@ -176,8 +175,6 @@ static void anaconda_mountpoint_selector_init(AnacondaMountpointSelector *mountp */ gtk_widget_set_can_focus(GTK_WIDGET(mountpoint), TRUE); gtk_widget_add_events(GTK_WIDGET(mountpoint), GDK_FOCUS_CHANGE_MASK|GDK_KEY_RELEASE_MASK);
g_signal_connect(mountpoint, "focus-in-event", G_CALLBACK(anaconda_mountpoint_selector_focus_changed), NULL);
g_signal_connect(mountpoint, "focus-out-event", G_CALLBACK(anaconda_mountpoint_selector_focus_changed), NULL);
/* Set "hand" cursor shape when over the selector */ mountpoint->priv->cursor = gdk_cursor_new(GDK_HAND2);
@@ -275,24 +272,6 @@ static void anaconda_mountpoint_selector_set_property(GObject *object, guint pro } }
-static gboolean anaconda_mountpoint_selector_focus_changed(GtkWidget *widget, GdkEventFocus *event, gpointer user_data) {
- GtkStateFlags new_state;
- new_state = gtk_widget_get_state_flags(widget) & ~GTK_STATE_FOCUSED;
- if (event->in) {
gtk_widget_show(GTK_WIDGET(ANACONDA_MOUNTPOINT_SELECTOR(widget)->priv->arrow));
new_state |= GTK_STATE_FOCUSED;
anaconda_mountpoint_selector_set_chosen(ANACONDA_MOUNTPOINT_SELECTOR(widget), TRUE);
- }
- else {
gtk_widget_hide(GTK_WIDGET(ANACONDA_MOUNTPOINT_SELECTOR(widget)->priv->arrow));
anaconda_mountpoint_selector_set_chosen(ANACONDA_MOUNTPOINT_SELECTOR(widget), FALSE);
- }
- gtk_widget_set_state_flags(widget, new_state, TRUE);
- return FALSE;
-}
static void anaconda_mountpoint_selector_toggle_background(AnacondaMountpointSelector *widget) { if (widget->priv->chosen) { gtk_widget_set_state_flags(GTK_WIDGET(widget), GTK_STATE_FLAG_SELECTED, FALSE); @@ -333,6 +312,12 @@ gboolean anaconda_mountpoint_selector_get_chosen(AnacondaMountpointSelector *wid void anaconda_mountpoint_selector_set_chosen(AnacondaMountpointSelector *widget, gboolean is_chosen) { widget->priv->chosen = is_chosen; anaconda_mountpoint_selector_toggle_background(widget);
- if (is_chosen)
- if (is_chosen) {
gtk_widget_show(GTK_WIDGET(widget->priv->arrow)); gtk_widget_grab_focus(GTK_WIDGET(widget));
- }
- else {
gtk_widget_hide(GTK_WIDGET(widget->priv->arrow));
- }
}
What about this patch? AFAICT the issue is still not resolved and I believe these changes are not so evil. :)
anaconda-patches@lists.fedorahosted.org