GNOME Bugzilla – Bug 778070
week-view: height of 30 minute events exceeds the tile.
Last modified: 2017-03-23 22:59:36 UTC
Created attachment 344766 [details] week-view 30 minute event (image) in week-view height of 30 minute events exceeds the tile. Please see the attached screenshot. Thanks
Created attachment 347805 [details] [review] week-grid: assign proper tile size to '30 minutes or less' events Earlier the size of the tile for an event which had a duration less than equal to 30 minutes exceeded the desired size. This patch fixes this sizing issue by assigning the correct size '33' to the event, which was earlier assigned wrong size '38'.
Created attachment 347812 [details] [review] week-grid: assign proper tile size to '30 minutes or less' events Earlier the size of the tile for an event which had a duration less than equal to 30 minutes exceeded the desired size. This patch fixes this sizing issue by assigning the correct size to the event, which was earlier assigned wrong size.
Created attachment 347823 [details] [review] week-grid: assign proper tile size to '30 minutes or less' events Earlier the size of the tile for an event which had a duration less than equal to 30 minutes exceeded the desired size. This patch fixes this sizing issue by assigning the correct size to the event, which was earlier assigned wrong size.
Created attachment 347826 [details] [review] week-grid: increase grid cell size The previous grid size was not enough for displaying the 30 minute or less events and they were "overflowing" into the next half hour. This fixes the problem by increasing the cell height by a ratio of padding.
Review of attachment 347823 [details] [review]: This is the wrong approach. The correct way to handle this is in GcalEventWidget, by checking if the available height is enough to fit the text. ::: src/views/gcal-week-grid.c @@ +721,3 @@ child_allocation.width = MAX (width, min_width); + /* 48/38 = 1.26 -- (height of 45 mins tile/ height of 30 mins tile) */ + child_allocation.height = (ABS(height / min_height_corrected) < 1.26) ? min_height_corrected : height; This is a bad idea. Those sizes work on a default GNOME environment, but e.g. change the font size and it won't be valid anymore.
Review of attachment 347826 [details] [review]: The approach here is also wrong. See the previous comment.
Created attachment 348451 [details] [review] event-widget: assign display-text in appropriate format to event-widget Earlier the height of event-widget with duration of 30 mins or less exceeded the desired space in week-grid. As height available was not checked beforehand, this resulted in some event-widgets getting height assigned which was more than the height available. This patch fixes the erroneous assignment of event-widget's height by first comparing the required height with the available height. If the required height exceeds the available height, then the display-text is given as a single-line for the widget to fit properly.
Created attachment 348452 [details] [review] week-grid: remove unneeded usage of natural-height As the assignment of display-text, and in turn that of event-widget's height, is taken care of in event-widget, we no longer need to consider natural- height, while assigning height in week-grid. This patch fixes this issue by removing natural-height, and directly assigning the desired value to height.
Review of attachment 348451 [details] [review]: Some commenta below. ::: src/gcal-event-widget.c @@ +134,3 @@ +static gint +get_maximum_height (GtkWidget *widget, + GtkOrientation orientation) Rename to 'get_vertical_text_height()'. You don't need the 'orientation' param here; just pass GTK_ORIENTATION_VERTICAL. @@ +580,3 @@ + /* As vertical widgets have the maximum height */ + max_height = get_maximum_height (widget, GTK_ORIENTATION_VERTICAL); Don't use a separate var here. Call 'get_vertical_text_height()' directly inside the if(). @@ +592,2 @@ { + display_text = get_visible_text (widget, GTK_ORIENTATION_HORIZONTAL); One line if()s don't need '{}'s @@ +595,3 @@ else { + display_text = get_visible_text (widget, GTK_ORIENTATION_VERTICAL); ditto
Review of attachment 348452 [details] [review]: Looks good.
Created attachment 348550 [details] [review] event-widget: assign display-text in appropriate format to event-widget Earlier the height of event-widget with duration of 30 mins or less exceeded the desired space in week-grid. As height available was not checked beforehand, this resulted in some event-widgets getting height assigned which was more than the height available. This patch fixes the erroneous assignment of event-widget's height by first comparing the required height with the available height. If the required height exceeds the available height, then the display-text is given as a single-line for the widget to fit properly.
Thanks for the patches! Attachment 348452 [details] pushed as 6cc6e4f - week-grid: remove unneeded usage of natural-height Attachment 348550 [details] pushed as 26ecf3c - event-widget: assign display-text in appropriate format to event-widget