After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 667104 - Borders between 3x3 blocks are too thin
Borders between 3x3 blocks are too thin
Status: RESOLVED FIXED
Product: gnome-sudoku
Classification: Applications
Component: general
git master
Other Linux
: Normal enhancement
: ---
Assigned To: gnome-sudoku-maint
gnome-sudoku-maint
Depends on:
Blocks:
 
 
Reported: 2012-01-01 20:15 UTC by caramba_bk
Modified: 2013-11-11 03:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Width of the sudoku grid changed (1.31 KB, patch)
2013-11-08 12:34 UTC, Pooja Prakash
committed Details | Review
Patch to thicken 3x3 block borders in vala-port branch (1.14 KB, patch)
2013-11-09 15:23 UTC, Parin Porecha
none Details | Review
Updated patch to thicken block borders in vala-port branch (1.14 KB, patch)
2013-11-09 15:39 UTC, Parin Porecha
needs-work Details | Review
Updated patch (changed border_width, corrected code-style mistakes) (1.19 KB, patch)
2013-11-09 18:28 UTC, Parin Porecha
committed Details | Review

Description caramba_bk 2012-01-01 20:15:47 UTC
The border between 3x3 blocks are too vague, they should be thicker since it's currently hard to distinguish which block a certain square belongs to. It would be best if this could be themable somehow, or just a settings key holding a scaling factor. If that approach is taken, it would also be nice to configure the brightness of the prefilled shaded squares.
Comment 1 Bob 2012-10-07 17:50:23 UTC
I love the Sudoku game, but agree with caramba that the border between the 3x3 blocks are too thin and that the prefilled shaded squares are too dark.  It would also be very helpful to streamline the approach to adding notes in squares by just typing more numbers in a square, instead of needing to open another window.

Thanks for giving us a great game.
Comment 2 Pooja Prakash 2013-10-26 14:31:28 UTC
Hi,

I am beginner in Open Source contribution. This bug seems interesting and I would like to work on it. Can someone please guide me on how to get started? Also can you please assign this bug to me.
Comment 3 Michael Catanzaro 2013-11-07 15:01:46 UTC
Hi Pooja, thanks for working on this!

We like to make separate bug reports for each issue, so the issue of shaded squares being too dark would be best dealt with in a new bug report. But in this case, I don't think that's necessary, because we're working on rewriting Sudoku in Vala, and the shaded squares are already lighter in that branch (vala-port).

I'd still be interested in seeing your patch for the borders, but I'd be more interested if it was for the Vala version. In that branch, the 3x3 blocks do not have thicker borders at all.
Comment 4 Michael Catanzaro 2013-11-07 17:02:40 UTC
It looks like Bug #676954 exists for the dark background, so we can discuss that there.
Comment 5 Pooja Prakash 2013-11-08 12:34:36 UTC
Created attachment 259248 [details] [review]
Width of the sudoku grid changed
Comment 6 Michael Catanzaro 2013-11-08 13:51:49 UTC
Hm, that actually looks a lot better than I was expecting. Thanks!

Attachment 259248 [details] pushed as 77d2b5c - Width of the sudoku grid changed
Comment 7 Parin Porecha 2013-11-09 15:23:05 UTC
Created attachment 259329 [details] [review]
Patch to thicken 3x3 block borders in vala-port branch

Hi,

I've attached the patch to thicken the borders of 3x3 blocks in vala-port branch.
Yes, this bug has been marked as RESOLVED, but I don't know where else to attach.
Comment 8 Parin Porecha 2013-11-09 15:39:45 UTC
Created attachment 259330 [details] [review]
Updated patch to thicken block borders in vala-port branch

Updated patch to work with 6x6 and 12x12 (and similar) sudokus, where no. of rows in a block != no. of columns in it
Comment 9 Michael Catanzaro 2013-11-09 16:59:34 UTC
(In reply to comment #7)
> Yes, this bug has been marked as RESOLVED, but I don't know where else to
> attach.

Here's fine, but future patches can go to Bug #633464.
Comment 10 Michael Catanzaro 2013-11-09 17:07:28 UTC
Review of attachment 259330 [details] [review]:

Thanks for working on this. This patch looks mostly good, but I have a couple suggestions.

::: src/sudoku-view.vala
@@ +604,3 @@
         grid.row_spacing = 1;
         grid.column_spacing = 1;
+        grid.border_width = 4;

I tried a couple different values and I think 3 looks better than 4, can we use that instead?

@@ +670,3 @@
+                if (col != 0 && (col % game.board.block_rows) == 0)
+                {
+                    cell.set_margin_left(4);

Two comments here.  First, in the Vala branch we leave a space between the function name and the opening parenthesis - it's a bit hard to remember at first, but you'll get used to it.  Second, we can just use grid.border_width here instead of writing the constant out a second and third time, so if in the future someone changes the width above, these will all change in sync. So this line (and the one below) could read:

cell.set_margin_left ((int) grid.border_width);
Comment 11 Parin Porecha 2013-11-09 18:28:50 UTC
Created attachment 259341 [details] [review]
Updated patch (changed border_width, corrected code-style mistakes)

I've corrected the mistakes, and updated the patch
Comment 12 Michael Catanzaro 2013-11-11 03:13:40 UTC
Review of attachment 259341 [details] [review]:

Pushed to vala-port, thank you!