GNOME Bugzilla – Bug 710626
Visual design updates
Last modified: 2014-08-13 09:22:58 UTC
https://raw.github.com/gnome-design-team/gnome-mockups/master/games/sudoku/sudoku.png Various visual improvements: * Use the same thickness of line between groups of squares (but use different colours) * Lighten the colour of completed squares * Change the colour and stroke thickness of the selected square
*** Bug 664985 has been marked as a duplicate of this bug. ***
(In reply to comment #0) > * Change the colour and stroke thickness of the selected square By chance, this used to look similar to the design, but after an Adwaita update it has become completely black, which is a big problem since it's our default theme. Parin, you'll need to change your theme and build GTK+ master with jhbuild to test this -- Adwaita recently moved from gnome-themes-standard to gtk+.
okay, I'm on it
Created attachment 280604 [details] [review] Visual design updates The patch contains fix for 2 of the suggested changes : * Use the same thickness of line between groups of squares (but use different colours) * Change the colour and stroke thickness of the selected square I haven't included 'Lighten the colour of completed squares' because right now, the board already has 3 colors - normal bg color, fixed cell bg color and highlighter color. With the possibility of tracker colors also to be included, the no. of resultant colors will become 5. I'm not sure if we want the user to comprehend so many colors for a simple game like this. But that's just my humble opinion. Let me know if you also want the 2nd suggestion to be included, I'll update the patch. btw, the board now looks like this - http://i.imgur.com/bBGzwrS.png I'm not very good with colors, so I've set the selected bg color as close as I could to the one in mockup.
Patch looks good, except that lambda is long enough that I would split it into its own function. Allan, could you look at that screenshot and let us know if this looks good to you?
Created attachment 280606 [details] [review] Visual design updates Moved the callback lambda to a separate function
Created attachment 280667 [details] [review] Visual design updates I had chat with Allan today, and this is what he suggested - - Lighten the black color used for block lines - Remove the 3px stroke inside the selected cell This patch has the fix for his suggestions. Here's what the board looks like post this update - http://i.imgur.com/lH4lB60.png
Review of attachment 280667 [details] [review]: OK
Attachment 280667 [details] pushed as aef2b5c
Ah I got a bit too confident here from your screenshot; it looks fine in the dark theme, but if you switch to the light theme (use GtkInspector!) you'll see there are no longer borders around the edge of the board. Other than that, much better!
From a quick skim of the draw_block_lines() function, I think you need to take another look at what's going on here. Besides the missing borders, it doesn't look like you're drawing enough lines inside the board, either. Maybe you were planning to make this a loop and just forgot?
I've set the row-spacing and column-spacing of the board to 1px, and set the background color of the board to the color of the lines we want b/w two intra-block cells. This removes the need to draw lines b/w cells. For inter-block, I'm drawing 4 lines - 2 vertical, 2 horizontal at 1/3 and 2/3 width and length of the board. This way we don't need a loop.
For adding a border, I'll need to change the background color to the dark one we use for inter-block. Then, I'll draw 12 lines - 6 vertical, 6 horizontal for intra-block cells. You may suggest drawing a dark colored rectangle along the board's border instead of this way, but I tried that and it doesn't work. (because max allowed dimensions <= dimensions of the widget)
(In reply to comment #12) > For inter-block, I'm drawing 4 lines - 2 vertical, 2 horizontal at 1/3 and 2/3 > width and length of the board. > This way we don't need a loop. OK, I see. It seems like it should work, but I am not seeing the second horizontal and vertical lines. Maybe they're being drawn over later? (And even the first lines don't show up when maximized.) (In reply to comment #13) > For adding a border, I'll need to change the background color to the dark one > we use for inter-block. Then, I'll draw 12 lines - 6 vertical, 6 horizontal for > intra-block cells. You mean 8, 4, and 4 (not 12, 6, and 6), right?
Created attachment 281397 [details] screenshot
> OK, I see. It seems like it should work, but I am not seeing the second > horizontal and vertical lines. Maybe they're being drawn over later? (And even > the first lines don't show up when maximized.) If you look very very closely in your screenshot, you can see that they are drawn, but they're off by 1px (centre block, look at fixed cell with value 6, you can see that the 2nd horizontal line should be 1px below). Reason is the double being off by a few decimal places on window size change. > (In reply to comment #13) > > For adding a border, I'll need to change the background color to the dark one > > we use for inter-block. Then, I'll draw 12 lines - 6 vertical, 6 horizontal for > > intra-block cells. > > You mean 8, 4, and 4 (not 12, 6, and 6), right? No, 6 vertical - 2 lines b/w 3 cells for each block, and 6 horizontal - again 2 for each block.
sorry, I didn't think this all the way through. This will cause the same problems as Arnaud mentioned in https://bugzilla.gnome.org/show_bug.cgi?id=733520#c7
Hey Parin, can you prioritize this bug before tracker (but after qqwing)? We can release without tracker if need be, but we can't release as long as things still look like comment #15. :)
(In reply to comment #18) > Hey Parin, can you prioritize this bug before tracker (but after qqwing)? We > can release without tracker if need be, but we can't release as long as things > still look like comment #15. :) Arnaud's DrawingArea patch (https://bugzilla.gnome.org/show_bug.cgi?id=733730#c9) mostly fixes this. I've rebased it against master and pushed it. Can you please test if it works for you as well ?
(In reply to comment #19) > Arnaud's DrawingArea patch > (https://bugzilla.gnome.org/show_bug.cgi?id=733730#c9) mostly fixes this. I've > rebased it against master and pushed it. Yup, looks good.
Great stuff!