GNOME Bugzilla – Bug 763198
Changing months in the month view using arrow keys.
Last modified: 2017-04-17 18:20:40 UTC
While using the arrow navigation in the month view, on reaching the end of the calendar, the selector doesn't move any further. This is an attempt to change that into switching between months. Steps to Reproduce: -Enter Month-view -Navigate towards the end of a calendar in any direction(say 1st of the month) -Press the Up arrow Actual Result: -There is no change in the position of the selector Expected Result: -The view should go to the previous month and the selector should be pointing towards the corresponding day which should be in the location if the months were joined.
Created attachment 323522 [details] [review] window: wrap around months for selector In this commit, we modify the position of the selector in the month view when the selector is out of bound of the present month Presently, the selector in the month view doesn't move when it reaches the border of the month. To fix this, whenever the keyboard_cell is less than first index of the month, we reduce the value of month by 1 and in the case it is greater than the max key cell, we are increasing the value of month by 1.
Review of attachment 323522 [details] [review]: The changed component changed was month-view, not window. Please update the commit message. More comments below: ::: src/gcal-month-view.c @@ +728,3 @@ { priv->keyboard_cell += diff; + }else if(priv->keyboard_cell + diff > max){ This code clearly disrespects the GNU C Coding Style. Read https://developer.gnome.org/programming-guidelines/stable/c-coding-style.html.en and update the patch. @@ +730,3 @@ + }else if(priv->keyboard_cell + diff > max){ + priv->date->month = priv->date->month + 1; + if(priv->date->month == 13){ Jump a line. @@ +733,3 @@ + priv->date->month = 1; + priv->date->year = priv->date->year + 1; + } Use icaltime_normalize() instead. See https://github.com/libical/libical/tree/master/src/libical documentation (specially icaltime.h). @@ +734,3 @@ + priv->date->year = priv->date->year + 1; + } + priv->days_delay = (time_day_of_week (1, priv->date->month - 1, priv->date->year) - priv->first_weekday + 7) % 7; Jump a line. @@ +736,3 @@ + priv->days_delay = (time_day_of_week (1, priv->date->month - 1, priv->date->year) - priv->first_weekday + 7) % 7; + priv->keyboard_cell = priv->days_delay + priv->keyboard_cell + diff - max - 1; + }else{ Ditto. @@ +738,3 @@ + }else{ + priv->date->month = priv->date->month-1; + if(priv->date->month == 0){ Ditto. @@ +741,3 @@ + priv->date->month = 12; + priv->date->year = priv->date->year - 1; + } Ditto. @@ +742,3 @@ + priv->date->year = priv->date->year - 1; + } + priv->days_delay = (time_day_of_week (1, priv->date->month - 1, priv->date->year) - priv->first_weekday + 7) % 7; Ditto.
Created attachment 323540 [details] [review] month-view: wrap around months for selector In this commit, we modify the position of the selector in the month view when the selector is out of bound of the present month Presently, the selector in the month view doesn't move when it reaches the border of the month. To fix this, whenever the keyboard_cell is less than first index of the month, we reduce the value of month by 1 and in the case it is greater than the max key cell, we are increasing the value of month by 1.
Review of attachment 323540 [details] [review]: Some nitpicks. ::: src/gcal-month-view.c @@ +733,3 @@ + priv->date->month = priv->date->month + 1; + *(priv->date) = icaltime_normalize(*(priv->date)); + priv->days_delay = (time_day_of_week (1, priv->date->month - 1, priv->date->year) - priv->first_weekday + 7) % 7; Jump a line. @@ +734,3 @@ + *(priv->date) = icaltime_normalize(*(priv->date)); + priv->days_delay = (time_day_of_week (1, priv->date->month - 1, priv->date->year) - priv->first_weekday + 7) % 7; + priv->keyboard_cell = priv->days_delay + priv->keyboard_cell + diff - max - 1; Jump a line and add a small comment explaining this line. @@ +740,3 @@ + priv->date->month = priv->date->month - 1; + *(priv->date) = icaltime_normalize(*(priv->date)); + priv->days_delay = (time_day_of_week (1, priv->date->month - 1, priv->date->year) - priv->first_weekday + 7) % 7; Jump a line. @@ +741,3 @@ + *(priv->date) = icaltime_normalize(*(priv->date)); + priv->days_delay = (time_day_of_week (1, priv->date->month - 1, priv->date->year) - priv->first_weekday + 7) % 7; + priv->keyboard_cell = priv->days_delay + icaltime_days_in_month (priv->date->month, priv->date->year) - min + priv->keyboard_cell + diff; Jump a line and add a small comment explaining this line.
Created attachment 323552 [details] [review] month-view: wrap around months for selector In this commit, we modify the position of the selector in the month view when the selector is out of bound of the present month Presently, the selector in the month view doesn't move when it reaches the border of the month. To fix this, whenever the keyboard_cell is less than first index of the month, we reduce the value of month by 1 and in the case it is greater than the max key cell, we are increasing the value of month by 1.
Created attachment 323553 [details] [review] Attachment to Bug 763198 - Changing months in the month view using arrow keys. month-view: wrap around months for selector In this commit, we modify the position of the selector in the month view when the selector is out of bound of the present month Presently, the selector in the month view doesn't move when it reaches the border of the month. To fix this, whenever the keyboard_cell is less than first index of the month, we reduce the value of month by 1 and in the case it is greater than the max key cell, we are increasing the value of month by 1.
Created attachment 323554 [details] [review] month-view: wrap around months for selector In this commit, we modify the position of the selector in the month view when the selector is out of bound of the present month Presently, the selector in the month view doesn't move when it reaches the border of the month. To fix this, whenever the keyboard_cell is less than first index of the month, we reduce the value of month by 1 and in the case it is greater than the max key cell, we are increasing the value of month by 1.
Review of attachment 323554 [details] [review]: More nitpicks. ::: src/gcal-month-view.c @@ +746,3 @@ + priv->days_delay = (time_day_of_week (1, priv->date->month - 1, priv->date->year) - priv->first_weekday + 7) % 7; + + /*Updating keyboard_cell value to the the displacement in the cell after entering successive month*/ This comment doesn't really explain what the following line does. I expect something like "Set the keyboard selection to the first day of the next month". Besides that, the comment doesn't respect GNU C Coding Standard. Add a blank space between '*' and the text. @@ +756,3 @@ + priv->days_delay = (time_day_of_week (1, priv->date->month - 1, priv->date->year) - priv->first_weekday + 7) % 7; + + /*Updating keyboard_cell value to the the displacement in the cell after entering preceding month*/ Ditto.
Created attachment 323587 [details] [review] month-view: wrap around months for selector In this commit, we modify the position of the selector in the month view when the selector is out of bound of the present month Presently, the selector in the month view doesn't move when it reaches the border of the month. To fix this, whenever the keyboard_cell is less than first index of the month, we reduce the value of month by 1 and in the case it is greater than the max key cell, we are increasing the value of month by 1.
Review of attachment 323587 [details] [review]: I just realized one more issue. Hopefullu, the last one before this patch is good enough. ::: src/gcal-month-view.c @@ +741,3 @@ + * the displacement from current cell and the max value in the current month. + */ + priv->keyboard_cell = priv->days_delay + priv->keyboard_cell + diff - max - 1; After this code, it should call g_object_notify() to notify GcalWindow about the property change. @@ +755,3 @@ + * minimum value in current month and the displacement from current cell. + */ + priv->keyboard_cell = priv->days_delay + icaltime_days_in_month (priv->date->month, priv->date->year) - min + priv->keyboard_cell + diff; Ditto.
Review of attachment 323587 [details] [review]: I just realized one more issue. Hopefully, the last one before this patch is good enough. ::: src/gcal-month-view.c @@ +741,3 @@ + * the displacement from current cell and the max value in the current month. + */ + priv->keyboard_cell = priv->days_delay + priv->keyboard_cell + diff - max - 1; After this code, it should call g_object_notify() to notify GcalWindow about the property change. @@ +755,3 @@ + * minimum value in current month and the displacement from current cell. + */ + priv->keyboard_cell = priv->days_delay + icaltime_days_in_month (priv->date->month, priv->date->year) - min + priv->keyboard_cell + diff; Ditto.
Created attachment 323731 [details] [review] month-view: wrap around months for selector In this commit, we modify the position of the selector in the month view when the selector is out of bound of the present month Presently, the selector in the month view doesn't move when it reaches the border of the month. To fix this, whenever the keyboard_cell is less than first index of the month, we reduce the value of month by 1 and in the case it is greater than the max key cell, we are increasing the value of month by 1.
The patch looks good, and works amazingly as well. Thanks for the fix. Attachment 323731 [details] pushed as a738acb - month-view: wrap around months for selector