Re: GtkCalendar improved keyboard focus movement

[Owen Taylor <otaylor redhat com>]

Detlef Reichl <detlef reichl arcormail de> writes:

> On Die, 2001-10-02 at 15:09, Owen Taylor wrote:
> > 
> > It's really best to submit patches to bugzilla
> bug 61655
> > 
> > Two comments about your patch:
> > 
> >  * The coding style needs to match that in GTK+. The GTK+
> >    coding style is basically that documented in 
> >    pango/docs/TEXT/coding-style, with the difference 
> >    that gint/gchar/getc. are used rather than int/char.
> fixed
> 
> > 
> >  * I think this needs to obey the GTK_CALENDAR_NO_MONTH change
> >    flag or you could seriously confuse some applications.
> oops. also fixed

Hmm, I looking more at the patch in detail, I found the
code rather confusing. (Not just your code, but also the
existing code.) So, I rewrote it so that all the changes
of the day went through a common code path. 

But trying out the result, I'm not sure it is a good change.

 * Changing months when you go into the next/previous month
   displayed dates strikes me as very confusing.

 * Changing months only when you go out of the range of displayed
   dates might work better, but I'm not completely convinced
   that it is superior to the current approach where
   Ctrl-arrows are needed to change months. It's easier to 
   figure out how to change months, but also easier to trigger
   accidentally.

Unfortunately, Calum's handy keybinding chart doesn't extend
to calendar widgets so we don't have any guidance there ;-)

Regards,
                                        Owen

Index: gtkcalendar.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkcalendar.c,v
retrieving revision 1.37
diff -u -p -r1.37 gtkcalendar.c
--- gtkcalendar.c	2001/09/20 19:35:08	1.37
+++ gtkcalendar.c	2001/10/03 18:13:18
@@ -1926,6 +1926,19 @@ gtk_calendar_paint_main (GtkWidget *widg
       gtk_calendar_paint_day (widget, row, col);
 }
 
+static gint
+first_day_column (GtkCalendar *calendar)
+{
+  gint first_day = day_of_week (calendar->year, calendar->month + 1, 1);
+  
+  if (calendar->display_flags & GTK_CALENDAR_WEEK_START_MONDAY)
+    first_day--;
+  else
+    first_day %= 7;
+
+  return first_day;
+}
+
 static void
 gtk_calendar_compute_days (GtkCalendar *calendar)
 {
@@ -1945,14 +1958,8 @@ gtk_calendar_compute_days (GtkCalendar *
   
   ndays_in_month = month_length[leap (year)][month];
   
-  first_day = day_of_week (year, month, 1);
+  first_day = first_day_column (calendar);
   
-  if (calendar->display_flags & GTK_CALENDAR_WEEK_START_MONDAY)
-    first_day--;
-  else
-    first_day %= 7;
-  
-  
   /* Compute days of previous month */
   if (month > 1)
     ndays_in_prev_month = month_length[leap (year)][month-1];
@@ -2595,14 +2602,83 @@ gtk_calendar_destroy (GtkObject *object)
     (* GTK_OBJECT_CLASS (parent_class)->destroy) (object);
 }
 
+/* Change the currently focused day by @increment.
+ * change the current month if the result is not in the
+ * current month and the calendar allows changing the month.
+ */
+static void
+adjust_focus_day (GtkCalendar *calendar,
+		  gint         increment)
+{
+  gint old_focus_row = calendar->focus_row;
+  gint old_focus_col = calendar->focus_col;
+  gint day;
+  gint ndays_in_prev_month;
+  gint ndays_in_this_month;
+  gint index;
+
+  ndays_in_this_month = month_length[leap (calendar->year)][calendar->month + 1];
+  if (calendar->month == 0)
+    ndays_in_prev_month = month_length[leap (calendar->year - 1)][12];
+  else
+    ndays_in_prev_month = month_length[leap (calendar->year)][calendar->month];
+
+  /* Find the day relative to the current font, using out-of-range
+   * values for adjacent months
+   */
+  day = calendar->day[calendar->focus_row][calendar->focus_col];
+  switch (calendar->day_month[calendar->focus_row][calendar->focus_col])
+    {
+    case MONTH_PREV:
+      day = day - ndays_in_prev_month;
+      break;
+    case MONTH_CURRENT:
+      break;
+    case MONTH_NEXT:
+      day = day + ndays_in_this_month;
+      break;
+    }
+
+  /* Adjust the day by the increment, change the month if necessary
+   */
+  day += increment;
+
+  if (!(calendar->display_flags & GTK_CALENDAR_NO_MONTH_CHANGE))
+    {
+      if (day <= 0)
+	{
+	  day += ndays_in_prev_month;
+	  
+	  gtk_calendar_set_month_prev (calendar);
+	}
+      else if (day > ndays_in_this_month)
+	{
+	  day -= ndays_in_this_month;
+	  
+	  gtk_calendar_set_month_next (calendar);
+	}
+    }
+
+  /* And focus the new day, if it is in the proper range
+   */
+  index = day + first_day_column (calendar) - 1;
+
+  if (index >= 0 && index < 42)
+    {
+      calendar->focus_row = index / 7;
+      calendar->focus_col = index % 7;
+      
+      gtk_calendar_paint_day (GTK_WIDGET (calendar), old_focus_row, old_focus_col);
+      gtk_calendar_paint_day (GTK_WIDGET (calendar), calendar->focus_row, calendar->focus_col);
+    }
+}
+
 static gboolean
 gtk_calendar_key_press (GtkWidget   *widget,
 			GdkEventKey *event)
 {
   GtkCalendar *calendar;
   gint return_val;
-  gint old_focus_row;
-  gint old_focus_col;
   gint row, col, day;
 
   g_return_val_if_fail (widget != NULL, FALSE);
@@ -2612,92 +2688,39 @@ gtk_calendar_key_press (GtkWidget   *wid
   calendar = GTK_CALENDAR (widget);
   return_val = FALSE;
   
-  old_focus_row = calendar->focus_row;
-  old_focus_col = calendar->focus_col;
-
   switch (event->keyval)
     {
     case GDK_KP_Left:
     case GDK_Left:
       return_val = TRUE;
       if (event->state & GDK_CONTROL_MASK)
-	{
-	  gtk_calendar_set_month_prev (calendar);
-	}
+	gtk_calendar_set_month_prev (calendar);
       else
-	{
-	  if (calendar->focus_col > 0)
-	    {
-	      calendar->focus_col--;
-	    }
-	  else if (calendar->focus_row > 0)
-	    {
-	      calendar->focus_col = 6;
-	      calendar->focus_row--;
-	    }
-	  gtk_calendar_paint_day (widget, old_focus_row, old_focus_col);
-	  gtk_calendar_paint_day (widget, calendar->focus_row,
-				  calendar->focus_col);
-	}
+	adjust_focus_day (calendar, -1);
       break;
     case GDK_KP_Right:
     case GDK_Right:
       return_val = TRUE;
       if (event->state & GDK_CONTROL_MASK)
-	{
 	gtk_calendar_set_month_next (calendar);
-	}
       else
-	{
-	  if (calendar->focus_col < 6)
-	    {
-	      calendar->focus_col++;
-	    }
-	  else if (calendar->focus_row < 5)
-	    {
-	      calendar->focus_col = 0;
-	      calendar->focus_row++;
-	    }
-	  gtk_calendar_paint_day (widget, old_focus_row, old_focus_col);
-	  gtk_calendar_paint_day (widget, calendar->focus_row,
-				  calendar->focus_col);
-	}
+	adjust_focus_day (calendar, 1);
       break;
     case GDK_KP_Up:
     case GDK_Up:
       return_val = TRUE;
       if (event->state & GDK_CONTROL_MASK)
-	{
-	  gtk_calendar_set_year_prev (calendar);
-	}
+	gtk_calendar_set_year_prev (calendar);
       else
-	{
-	  if (calendar->focus_row > 0)
-	    {
-	      calendar->focus_row--;
-	    }
-	  gtk_calendar_paint_day (widget, old_focus_row, old_focus_col);
-	  gtk_calendar_paint_day (widget, calendar->focus_row,
-				  calendar->focus_col);
-	}
+	adjust_focus_day (calendar, -7);
       break;
     case GDK_KP_Down:
     case GDK_Down:
       return_val = TRUE;
       if (event->state & GDK_CONTROL_MASK)
-	{
-	  gtk_calendar_set_year_next (calendar);
-	}
+	gtk_calendar_set_year_next (calendar);
       else
-	{
-	  if (calendar->focus_row < 5)
-	    {
-	      calendar->focus_row++;
-	    }
-	  gtk_calendar_paint_day (widget, old_focus_row, old_focus_col);
-	  gtk_calendar_paint_day (widget, calendar->focus_row,
-				  calendar->focus_col);
-	}
+	adjust_focus_day (calendar, 7);
       break;
     case GDK_KP_Space:
     case GDK_space: