Re: [evolution-patches] Bug 46351, Calendar:Accesskey for Time_z_one doesn't work

[Yuedong Du <yuedong du sun com>]

Several problems:

1. The coding style still not good enough. I know you have improved a 
lot since the first patch. But it can be better. for example

+		if(!GTK_WIDGET_HAS_FOCUS (widget))
		  |
	place a space here 
Please read the 'Coding style' section in evolution/HACKING by notzed.

2. Why there are 2 times of GTK_WIDGET_SET_FLAGS() call in your e_timezone_entry_mnemonic_activate()? I think at least one
can be removed, right?

3. I don't think you need to set GTK_WIDGET_CAN_FOCUS on a button. It is already initialized in button's constructor.

4. And I don't like the code below, although I know you copied it from gtk+,

+	} else {
+		g_warning ("widget `%s' isn't suitable for mnemonic activation",
+		G_OBJECT_TYPE_NAME (widget));
+		gdk_display_beep (gtk_widget_get_display (widget));
+	}


Can you investigate on if it can be avoided by call parent's method?






Regards,
York



Carl sun wrote:

>    This function (/get_private_buttons/) won't be be called from the 
> a11y directory/,/instead it is called by another function in my patch:
>     e_timezone_entry_mnemonic_activate (GtkWidget *widget, gboolean   
> group_cycling).
>  
>    Now, I will give up this function(/get_private_buttons/).and Please 
> check my new version of this patch, and get it from the attachment!
>
>    Thank you!
>
>    Carl Sun
>
> On Thu, 2003-11-27 at 15:19, JP Rosevear wrote:
>
> > /On Mon, 2003-11-24 at 02:43, Carl sun wrote:
> > > Hi, JP Rosevear,
> > >
> > >      I am explaining why I add get_private_buttons accessor method  in
> > > my submitted patch:
> > >      As you known, "button"  is defined in the ETimezoneEntryPrivate,
> > > a private attribute of ETimezoneEntry.  The function 
> > > e_timezone_entry_mnemonic_activate (GtkWidget *widget,     gboolean 
> > > group_cycling) doesn't belong to member method of ETimezoneEntry.
> > > According to encapsulation and accessive mechanism of OO,  so I add
> > > the member method "get_private_buttons" to access the private
> > > attribute of ETimezoneEntry.
> > >
> > >     Of course, I have tried deleting this accessor method, that is
> > > okay. but I don't think this way works better for the sake of  the
> > > complexity of access and breaking the encapsulation mechanism.
> > >    
> > >     It's up to you which way we can take. If the latter is much
> > > prefer, I will send the new version of patch to you. Waiting for your
> > > reply! 
> >
> > Maybe I'm missing something - no where in this patch do you seem to call
> > the get_private_buttons accessor.  Is this going to be called from the
> > a11y directory?  Please note that the a11y patches are supposed to be
> > approved by one of the other a11y guys as well.
> >
> > -JP/
> ------------------------------------------------------------------------
>
> Index: ChangeLog
> ===================================================================
> RCS file: /export/src/cvs/evolution/calendar/ChangeLog,v
> retrieving revision 1.6
> diff -u -r1.6 ChangeLog
> --- ChangeLog	2003/10/20 09:20:34	1.6
> +++ ChangeLog	2003/11/27 09:20:14
> @@ -1,3 +1,15 @@
> +2003-11-11 Carl Sun <carl sun sun com>
> +                                                                                
> +        Fixes #46351
> +                                                                                
> +        * gui/e-timezone-entry.c  (e_timezone_entry_mnemonic_activate): 
> +          new function. override the  member function of GtkWidget to handle  
> +          mnemonic_activate signal of custom class ETimezoneEntry. 
> +        (e_timezone_entry_init):set the flags of private button to  
> +          GTK_CAN_FOCUSE|GTK_CAN_DEFAULT
> +        (e_timezone_entry_new):set the flags of ETimezoneEntry to GTK_CAN_FOCUS
> +        (e_timezone_entry_class_init):set the function pointer of GtkWidget's 
> +          mnemonic_activate to e_timezone_entry_mnemonic_activate.
>
> -----------------tag A11Y_M2--------------
>
> Index: gui/e-timezone-entry.c
> ===================================================================
> RCS file: /export/src/cvs/evolution/calendar/gui/e-timezone-entry.c,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1 e-timezone-entry.c
> --- gui/e-timezone-entry.c	2003/09/26 06:34:29	1.1.1.1
> +++ gui/e-timezone-entry.c	2003/11/27 09:20:14
> @@ -69,6 +69,8 @@
> static void e_timezone_entry_init	(ETimezoneEntry	*tentry);
> static void e_timezone_entry_destroy	(GtkObject	*object);
>
> +static gboolean e_timezone_entry_mnemonic_activate (GtkWidget *widget,
> +                                                    gboolean   group_cycling);
> static void on_entry_changed		(GtkEntry	*entry,
> 					 ETimezoneEntry *tentry);
> static void on_button_clicked		(GtkWidget	*widget,
> @@ -92,6 +94,9 @@
>
> 	parent_class = g_type_class_peek_parent (class);
>
> +	GtkWidgetClass *widget_class = GTK_WIDGET_CLASS (class) ;
> +	widget_class->mnemonic_activate = e_timezone_entry_mnemonic_activate;
> +
> 	timezone_entry_signals[CHANGED] =
> 		gtk_signal_new ("changed",
> 				GTK_RUN_LAST,
> @@ -130,6 +135,7 @@
> 	g_signal_connect (priv->entry, "changed", G_CALLBACK (on_entry_changed), tentry);
> 	
> 	priv->button = gtk_button_new ();
> +	GTK_WIDGET_SET_FLAGS (GTK_WIDGET (priv->button), GTK_CAN_FOCUS|GTK_CAN_DEFAULT);
> 	g_signal_connect (priv->button, "clicked", G_CALLBACK (on_button_clicked), tentry);
> 	gtk_box_pack_start (GTK_BOX (tentry), priv->button, FALSE, FALSE, 6);
> 	gtk_widget_show (priv->button);
> @@ -158,6 +164,8 @@
>
> 	tentry = g_object_new (e_timezone_entry_get_type (), NULL);
>
> +	GTK_WIDGET_SET_FLAGS (GTK_WIDGET(tentry), GTK_CAN_FOCUS);
> +
> 	return GTK_WIDGET (tentry);
> }
>
> @@ -297,3 +305,31 @@
> 	g_free (name_buffer);
> }
>
> +
> +static gboolean
> +e_timezone_entry_mnemonic_activate (GtkWidget *widget,
> +                                    gboolean   group_cycling)
> +{
> +	GtkContainer *container;
> +	GtkButton *button = NULL;
> +	container = GTK_CONTAINER (widget);
> +
> +	if (!group_cycling && GTK_WIDGET_GET_CLASS (widget)->activate_signal)
> +		gtk_widget_activate (widget);
> +	else if (GTK_WIDGET_CAN_FOCUS (widget)) {
> +		if(!GTK_WIDGET_HAS_FOCUS (widget))
> +			GTK_WIDGET_SET_FLAGS (widget, GTK_HAS_FOCUS); 
> +                button=((ETimezoneEntryPrivate*) ((ETimezoneEntry*) widget)->priv)->button;
> +		if (button) {
> +			GTK_WIDGET_SET_FLAGS (button, GTK_HAS_FOCUS|GTK_CAN_FOCUS);
> +			gtk_container_set_focus_child (GTK_CONTAINER (widget), GTK_WIDGET (button));
> +			gtk_widget_grab_focus (button);
> +		} else
> +			gtk_widget_grab_focus (widget);
> +	} else {
> +		g_warning ("widget `%s' isn't suitable for mnemonic activation",
> +		G_OBJECT_TYPE_NAME (widget));
> +		gdk_display_beep (gtk_widget_get_display (widget));
> +	}
> +	return TRUE;
> +}
>