[PATCH 1/3] LayoutIndicator widget

Vratislav Podzimek vpodzime at redhat.com
Thu May 2 10:05:37 UTC 2013


On Tue, 2013-04-30 at 11:37 -0400, Chris Lumens wrote:
> >  widgets/configure.ac              |   1 +
> >  widgets/glade/AnacondaWidgets.xml |  10 +
> >  widgets/src/LayoutIndicator.c     | 430 ++++++++++++++++++++++++++++++++++++++
> >  widgets/src/LayoutIndicator.h     |  71 +++++++
> >  widgets/src/Makefile.am           |   8 +-
> 
> Make sure to add your new C file to po/POTFILES.in.
Done.

> 
> > +static void anaconda_layout_indicator_init(AnacondaLayoutIndicator *self) {
> > +    GdkDisplay *display;
> > +    GdkRGBA background_color = { 0.0, 0.0, 0.0, 0.0 };
> > +
> > +    g_return_if_fail(gdk_rgba_parse(&background_color, "#fdfdfd"));
> > +
> > +    self->priv = G_TYPE_INSTANCE_GET_PRIVATE(self,
> > +                                             ANACONDA_TYPE_LAYOUT_INDICATOR,
> > +                                             AnacondaLayoutIndicatorPrivate);
> > +
> > +    /* layout indicator should not change focus when it is clicked */
> > +    gtk_widget_set_can_focus(GTK_WIDGET(self), FALSE);
> > +
> > +    /* layout indicator should have a tooltip saying what is the current layout
> > +       and what clicking it does
> > +    */
> > +    gtk_widget_set_has_tooltip(GTK_WIDGET(self), TRUE);
> > +
> > +    /* layout indicator activates next layout when it is clicked */
> > +    gtk_widget_add_events(GTK_WIDGET(self), GDK_BUTTON_RELEASE_MASK);
> > +    g_signal_connect(self, "button-release-event",
> > +                     G_CALLBACK(anaconda_layout_indicator_clicked),
> > +                     NULL);
> 
> Is this widget usable with just the keyboard?
It's not, but I'm not sure we want something like that. The primary
purpose of the widget is to indicate which layout is currently
activated. Keyboard switching should be done by the shortcut (like
Alt+Shift) and the widget intentionally doesn't grab focus (which would
be needed to make it usable just with the keyboard), so that it can be
clicked with the focus kept in the text field that is being edited.

> 
> > +    /* layout indicator should have a different background color
> > +       TODO: should be "exported" to allow changes in glade from code? */
> > +    gtk_widget_override_background_color(GTK_WIDGET(self),
> > +                                         GTK_STATE_FLAG_NORMAL, &background_color);
> 
> I really only have two major complaints with this new widget, and this
> is one.  I think the white background makes it look weird and
> unfinished.  I would much rather find some other way to make it look
> activatable while still fitting in better with how everything else
> looks.
Well, this was actually Máirín's idea and I must say I like it. The
other way to make the widget compact and looking activatable could be a
frame around it [1], but that looks to me really old school with the
grey background. Any other ideas?

[1] http://vpodzime.fedorapeople.org/layout_indicator_old_school.png

> 
> > +    /* create and populate the main box */
> > +    self->priv->main_box = GTK_BOX(gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 4));
> > +    gtk_box_pack_start(self->priv->main_box, self->priv->icon, FALSE, FALSE, 0);
> > +    gtk_box_pack_end(self->priv->main_box, GTK_WIDGET(self->priv->layout_label), FALSE, FALSE, 0);
> > +    gtk_widget_set_margin_left(GTK_WIDGET(self->priv->main_box), 4);
> > +    gtk_widget_set_margin_right(GTK_WIDGET(self->priv->main_box), 4);
> > +    gtk_widget_set_margin_top(GTK_WIDGET(self->priv->main_box), 3);
> > +    gtk_widget_set_margin_bottom(GTK_WIDGET(self->priv->main_box), 3);
> 
> We've tried to use multiples of 6 for margins and box spacing wherever
> possible.  Using 3 is probably fine if you need things to look closer,
> but I'd rather the 4 be either a 3 or a 6, your choice.
Changed to 3 and 6.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list