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 710624 - Remove menu bar and toolbar
Remove menu bar and toolbar
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: 633464
Blocks:
 
 
Reported: 2013-10-22 10:41 UTC by Allan Day
Modified: 2014-05-25 13:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Screenshot of current "progress" on Vala port (27.55 KB, image/png)
2014-02-22 17:25 UTC, Michael Catanzaro
  Details
Screenshot showing the utilities dialog (51.56 KB, image/png)
2014-02-24 12:43 UTC, Parin Porecha
  Details
Patch to remove New Game Screen (22.21 KB, patch)
2014-05-18 06:15 UTC, Parin Porecha
none Details | Review
Updated patch to remove New Game Screen (22.67 KB, patch)
2014-05-18 07:26 UTC, Parin Porecha
committed Details | Review
Moved puzzle difficulty to HeaderBar subtitle, and made difficulty strings translatable (1.71 KB, patch)
2014-05-19 06:03 UTC, Parin Porecha
committed Details | Review
Patch to remove 'Hint' and 'Help' buttons and related code (23.12 KB, patch)
2014-05-20 12:19 UTC, Parin Porecha
committed Details | Review
Remove Undo and Redo options from Appmenu (3.93 KB, patch)
2014-05-22 10:58 UTC, Parin Porecha
reviewed Details | Review
Add Undo and Redo buttons in headerbar (6.19 KB, patch)
2014-05-22 11:06 UTC, Parin Porecha
reviewed Details | Review
Remove Undo and Redo options from Appmenu (3.38 KB, patch)
2014-05-23 10:26 UTC, Parin Porecha
accepted-commit_now Details | Review
Add Undo and Redo buttons in Headerbar (6.18 KB, patch)
2014-05-23 10:32 UTC, Parin Porecha
none Details | Review
Bump required GTK+ version to 3.13.0 (798 bytes, patch)
2014-05-23 10:34 UTC, Parin Porecha
accepted-commit_now Details | Review
Add Undo and Redo buttons in Headerbar (6.11 KB, patch)
2014-05-23 12:06 UTC, Parin Porecha
accepted-commit_now Details | Review

Description Allan Day 2013-10-22 10:41:11 UTC
Menus and toolbars aren't really suitable for games. They're not fun, and they don't prioritise the part of the game that people are interested in - namely, the game.

It is possible to items from the menus and toolbar to a combination of the app menu and in-game controls. Some suggestions:

 * Print, Help and About can move to the app menu
 * New game can be a button inside the window

I think that many of the other options need to be reconsidered. Many of the additional tools (hints, highlighter, show possible numbers) seem a bit unclear or lacking in utility. It might be a better approach to focus on getting the basics right.

https://raw.github.com/gnome-design-team/gnome-mockups/master/games/sudoku/sudoku.png
Comment 1 Michael Catanzaro 2013-10-22 15:35:43 UTC
This is completed in the Vala port.  It needs some more work before it can be merged.
Comment 2 Michael Catanzaro 2014-02-22 17:24:45 UTC
Let's try to decide what exactly is going to be removed and what will stay. From Parin's additions to our TODO:

- Sudoku master has total 19 menu options (8 in 'Game', 5 in 'Settings', 4 in 'Tools', 2 in 'Help')
  Current vala-port design and mockups have only one panel menu and it cannot accomodate all the 19 options
  Ask aday about this.

The current menu structure is:

- Game
--- New
--- Reset
--- Undo
--- Redo
--- Puzzle Statistics...
--- Print...
--- Print Multiple Sudokus...
--- Close

I think Undo/Redo belong in the window somewhere, not the app menu.  I've been moving these into the window in the other games. Not sure about Reset: in Mahjongg we relegate this to the app menu.

Both Prints are already implemented and surely get to stay.  Puzzle Statistics seems harmless, but I'm not sure where to put it (app menu?) and is probably optional.

- Settings
--- Fullscreen
--- Show Toolbar
--- Highlighter
--- Show Possible Numbers
--- Warn About Unfillable Squares

Fullscreen has been removed from most of the other games, since it doesn't look good unless both the window and the art is designed to scale nicely. I'm also not fully convinced of the benefit over simply maximizing the window now that we have header bars. If we want to keep fullscreen, then we can probably pack the fullscreen icon into the header bar on the left.

Show Toolbar obviously will be removed.

Highlighter and Show Possible Numbers have been brought up for possible removal.  Highlighter makes it easier to see which squares conflict with the current one, but it sure it... colorful.  Show Possible Numbers makes the game 13452356x easier. Warn About Unfillable Squares is a preference we should probably keep. We can put these into a small preferences dialog, or simply as toggles in the application menu (but the app menu is already looking awfully big). I don't think these belong in the window.

- Tools
--- Hint
--- Clear Top Notes
--- Clear Bottom Notes
--- Track Additions

Should probably keep Hint in the window somewhere. If we remove notes from the cells, two of these go away.

If we keep Track Additions, we need to figure out how to simplify it: it's currently too confusing.

- Help
--- Contents
--- About

These go at the bottom of the application menu; no issues here.

If we keep enough of the stuff in Settings, we can move it to a preferences dialog.
 
Also from TODO:

- Mockups have alloted a separate place for notes.
  This is totally different than the current design (top, bottom notes for each cell).
  Ask aday about this.

I'm also concerned about this.  I'm not sure that notes would be very useful if they're moved outside of the cells.

So let's figure out:

* What do we do about Notes?
* Remove the number pad?
* Back in redundant with Undo. We also want Redo in the window somewhere, and also Hint. Where do these go?
* The current design has a Start Again button. I guess this is equivalent to Restart Game?  Then you have to use the app menu to start a brand new game: is that desirable for a puzzle game where you might want to give up on the current puzzle?  Tetravex has a Resolve button, for example.
Comment 3 Michael Catanzaro 2014-02-22 17:25:17 UTC
Created attachment 269997 [details]
Screenshot of current "progress" on Vala port
Comment 4 Parin Porecha 2014-02-24 12:43:21 UTC
Created attachment 270115 [details]
Screenshot showing the utilities dialog

> Both Prints are already implemented and surely get to stay.  Puzzle Statistics
> seems harmless, but I'm not sure where to put it (app menu?) and is probably
> optional.
> 
> - Settings
> --- Fullscreen
> --- Show Toolbar
> --- Highlighter
> --- Show Possible Numbers
> --- Warn About Unfillable Squares
> 
> Fullscreen has been removed from most of the other games, since it doesn't look
> good unless both the window and the art is designed to scale nicely. I'm also
> not fully convinced of the benefit over simply maximizing the window now that
> we have header bars. If we want to keep fullscreen, then we can probably pack
> the fullscreen icon into the header bar on the left.
> 
> Show Toolbar obviously will be removed.
> 
> Highlighter and Show Possible Numbers have been brought up for possible
> removal.  Highlighter makes it easier to see which squares conflict with the
> current one, but it sure it... colorful.  Show Possible Numbers makes the game
> 13452356x easier. Warn About Unfillable Squares is a preference we should
> probably keep. We can put these into a small preferences dialog, or simply as
> toggles in the application menu (but the app menu is already looking awfully
> big). I don't think these belong in the window.
>

We can add an option 'Utilites' to the app menu.
Clicking on it will open a dialog with options to enable tools and to view the puzzle statistics.
I think showing the statistics there will help the user decide which options he wants to enable.
If the difficulty is too hard or the no. of moves instantly fillable by elimination/filling are very less, then he can decide there itself whether to enable 'Show possible numbers' or not.
I've attached the screenshot of the possible dialog (Just to give an idea about how it will look like)
 
> - Tools
> --- Hint
> --- Clear Top Notes
> --- Clear Bottom Notes
> --- Track Additions
> 
> Should probably keep Hint in the window somewhere. If we remove notes from the
> cells, two of these go away.

Hint button can be added above 'Start Again' in the current mockup.

> If we keep enough of the stuff in Settings, we can move it to a preferences
> dialog.
> 
> Also from TODO:
> 
> - Mockups have alloted a separate place for notes.
>   This is totally different than the current design (top, bottom notes for each
> cell).
>   Ask aday about this.
> 
> I'm also concerned about this.  I'm not sure that notes would be very useful if
> they're moved outside of the cells.

Now that we have popovers, notes for each cell will certainly look better than the textarea used in master.

> So let's figure out:
> 
> * What do we do about Notes?
> * Remove the number pad?

The number pad on the right would not be that useful since it is faster and easier to just input the no. from the pad which pops up below the cell. Removing it would be a good idea.

> * Back in redundant with Undo. We also want Redo in the window somewhere, and
> also Hint. Where do these go?

Top buttons can be Undo and Redo (do we really need labels for them ? Nautilus, internet browsers etc. use only icons for Back/Forward and they are perfect for it). They will be side-by-side taking total space of one button.
Below 'Hint' can be placed, and at the bottom 'Restart'
something like -

|  <  |  >  |
|   Hint    |
| Restart |
Comment 5 Michael Catanzaro 2014-02-24 15:37:08 UTC
(In reply to comment #4)
> We can add an option 'Utilites' to the app menu.
> Clicking on it will open a dialog with options to enable tools and to view the
> puzzle statistics.
> I think showing the statistics there will help the user decide which options he
> wants to enable.
> If the difficulty is too hard or the no. of moves instantly fillable by
> elimination/filling are very less, then he can decide there itself whether to
> enable 'Show possible numbers' or not.
> I've attached the screenshot of the possible dialog (Just to give an idea about
> how it will look like)

I think it would probably be better to have a preferences dialog that only focuses on preferences, with no extra information.  We can have another way to get to the game statistics if desired.
Comment 6 Michael Catanzaro 2014-02-24 15:39:26 UTC
(In reply to comment #4)
> Top buttons can be Undo and Redo (do we really need labels for them ? Nautilus,
> internet browsers etc. use only icons for Back/Forward and they are perfect for
> it).

I don't know. Normally we just use icons, but if the other buttons have text labels then those might be appropriate. Allan knows how to make things look good and he'll correct us if we get it wrong.
Comment 7 Michael Catanzaro 2014-03-09 16:23:47 UTC
(In reply to comment #2)
> Show Possible Numbers makes the game
> 13452356x easier. 

I'm now pretty convinced this is harmful to gameplay and shouldn't be re-implemented.
Comment 8 Mario Wenzel 2014-03-09 20:58:13 UTC
I used this for testing and if we could keep that behind a command line switch, that would be great.
Comment 9 Michael Catanzaro 2014-03-10 03:56:28 UTC
(In reply to comment #8)
> I used this for testing and if we could keep that behind a command line switch,
> that would be great.

That makes sense
Comment 10 Parin Porecha 2014-05-18 06:15:17 UTC
Created attachment 276714 [details] [review]
Patch to remove New Game Screen

This patch removes the new game screen.

If a savegame is present, it will be started on launching Sudoku.
If not, a new game with random difficulty will be started
Comment 11 Parin Porecha 2014-05-18 07:26:07 UTC
Created attachment 276717 [details] [review]
Updated patch to remove New Game Screen

On starting a new game, there wasn't a way to know which difficulty is it of.
Now, the HeaderBar title will be updated to show 'Sudoku - Easy', 'Sudoku - Hard' and similar strings.
Comment 12 Michael Catanzaro 2014-05-18 15:00:45 UTC
Review of attachment 276717 [details] [review]:

Since we're removing the new game screen, Sudoku needs to use GSettings to remember the previous difficulty, and start all new games with this difficulty.  We clearly need some way to change difficulty. The current designs don't accommodate this. Allan, can you help us?

Brainstorming: maybe we don't need to completely remove the new game screen, but can instead re-purpose it as a Change Difficulty screen.  We can show it when the user clicks a button in the window, on the bottom right, labeled Change Difficulty.

::: src/gnome-sudoku.vala
@@ +134,3 @@
         var rating = rater.get_difficulty ();
         rating.pretty_print ();
+        header_bar.title = "Sudoku - %s".printf (rating.get_catagory ().to_string ());

A couple notes:

* This needs to be translatable. Try something like "%s - %s.".printf (_("Sudoku"), rating.get_category...)
* Can you please modify the to_string() function to return the difficulty level in title case ("Very Hard" instead of "Very hard")?  (Also check to make sure that is translatable as well.)

I think it might look better for the title to be simply Sudoku, and to put the difficulty level in the subtitle instead.
Comment 13 Michael Catanzaro 2014-05-18 15:04:47 UTC
Comment on attachment 276717 [details] [review]
Updated patch to remove New Game Screen

Pushing since this is a major improvement, but note the review comments above.
Comment 14 Parin Porecha 2014-05-19 05:25:00 UTC
We also need to decide how the Undo/Redo and Restart buttons will be accommodated in the game window.

Plus, have we finalized whether to use one handwritten note for a game ?
The fixed number pad to the right of the board will be removed, and a textarea will be placed there (according to the mockups). So, if we're not going to use a note, then what should be shown there ?
Allan, suggestions ?
Comment 15 Parin Porecha 2014-05-19 06:03:05 UTC
Created attachment 276742 [details] [review]
Moved puzzle difficulty to HeaderBar subtitle, and made difficulty strings translatable
Comment 16 Michael Catanzaro 2014-05-19 14:12:38 UTC
(In reply to comment #14)
> We also need to decide how the Undo/Redo and Restart buttons will be
> accommodated in the game window.

I think we can place these in the start of the header bar. That's normally the upper-left, but in RTL layouts it will be the upper right.

The images for these buttons have to be set in the code, not in a UI file, because we have to use different images if we're running in a RTL layout. Chess is probably the best example of how to do this:

var undo_move_image = (Gtk.Image) builder.get_object ("undo_move_image");
bool rtl = Gtk.Widget.get_default_direction () == Gtk.TextDirection.RTL;
undo_move_image.icon_name = rtl ? "edit-undo-rtl-symbolic" : "edit-undo-symbolic";

> Plus, have we finalized whether to use one handwritten note for a game ?
> The fixed number pad to the right of the board will be removed, and a textarea
> will be placed there (according to the mockups). So, if we're not going to use
> a note, then what should be shown there ?
> Allan, suggestions ?

I do want to remove that number pad. I don't want to implement the single note on the right.

My suggestion is to place New Game, Start Over, and Change Difficulty buttons starting from the bottom-right and working up, similar to what we're doing in Mines 13.1. I'll ping Allan to see if he has a better plan.
Comment 17 Michael Catanzaro 2014-05-19 15:06:55 UTC
Let's also remove the Hint and Help buttons (and the code that's backing them).
Comment 18 Parin Porecha 2014-05-20 12:19:49 UTC
Created attachment 276851 [details] [review]
Patch to remove 'Hint' and 'Help' buttons and related code

'Back' button has also been renamed to 'Undo'.
Comment 19 Michael Catanzaro 2014-05-20 18:59:20 UTC
Good, though in the future, note that this should have been three separate patches.
Comment 20 Parin Porecha 2014-05-22 10:58:59 UTC
Created attachment 276976 [details] [review]
Remove Undo and Redo options from Appmenu

Also removed 'Undo' button below the number picker.

Please commit this patch first.
Comment 21 Parin Porecha 2014-05-22 11:06:52 UTC
Created attachment 276977 [details] [review]
Add Undo and Redo buttons in headerbar

These buttons will become insensitive when the respective stacks are empty.
You may find a little strange behavior when resetting the game (Even if the board is empty and you reset the game, 'Undo' will still remain sensitive), but it's not a bug in the button related code, it's because SudokuGame.reset () doesn't check if a cell is empty or not.
Tell me if you find any bugs, I'll include their fix with the one for this.

btw, since this bug's been marked FIXED, should I post the next patches someplace else ?
Comment 22 Michael Catanzaro 2014-05-22 14:13:40 UTC
(In reply to comment #21)
> btw, since this bug's been marked FIXED, should I post the next patches
> someplace else ?

I actually only closed this bug by accident.  But I think it'd be good to use the existing Vala port bug from now on.  This one is titled "remove menu bar and toolbar," which was completed a long time ago.
Comment 23 Michael Catanzaro 2014-05-22 14:24:07 UTC
Review of attachment 276976 [details] [review]:

::: src/gnome-sudoku.vala
@@ -32,3 @@
         {"reset", reset_cb                                          },
-        {"undo", undo_cb                                            },
-        {"redo", redo_cb                                            },

I think it'd be better to retain these in the action map, and associate these actions with the new buttons you're adding.  Quick tutorial coming in the next review.
Comment 24 Michael Catanzaro 2014-05-22 14:30:46 UTC
Review of attachment 276977 [details] [review]:

::: data/gnome-sudoku.ui
@@ +17,3 @@
+                        <property name="tooltip-text" translatable="yes">Undo your last action</property>
+                        <property name="can_focus">True</property>
+                        <property name="focus_on_click">False</property>

Here, add <property name="action-name">app.undo</property>

::: src/gnome-sudoku.vala
@@ +23,3 @@
 
+    private Button undo_button;
+    private Button redo_button;

You won't need these when using GAction, since you won't have to manually add any callbacks to the buttons or change their sensitivity.

@@ +126,3 @@
         header_bar.set_subtitle ("%s".printf (rating.get_catagory ().to_string ()));
+        undo_button.sensitive = false;
+        redo_button.sensitive = false;

Instead of changing the sensitivity of the buttons directly, you can change the sensitivity of the actions associated with them:

lookup_action (undo).set_enabled (false)
Comment 25 Michael Catanzaro 2014-05-22 21:56:00 UTC
Review of attachment 276977 [details] [review]:

::: src/gnome-sudoku.vala
@@ +92,3 @@
+        bool rtl = Gtk.Widget.get_default_direction () == Gtk.TextDirection.RTL;
+        undo_image.icon_name = rtl ? "edit-undo-rtl-symbolic" : "edit-undo-symbolic";
+        redo_image.icon_name = rtl ? "edit-redo-rtl-symbolic" : "edit-redo-symbolic";

I know this is exactly what I asked you to do yesterday... but take a look at Bug #730598.  Looks like you can avoid this mess by bumping the GTK+ version. (I've taken note to use this in other games soon.)
Comment 26 Parin Porecha 2014-05-23 10:26:17 UTC
Created attachment 277044 [details] [review]
Remove Undo and Redo options from Appmenu

Patch updated
Comment 27 Parin Porecha 2014-05-23 10:32:26 UTC
Created attachment 277046 [details] [review]
Add Undo and Redo buttons in Headerbar

Patch Updated.

Using the ActionMap's a nice suggestion.
Thanks ! learnt a new thing today.
Comment 28 Parin Porecha 2014-05-23 10:34:24 UTC
Created attachment 277048 [details] [review]
Bump required GTK+ version to 3.13.0

From now, I'll post patches to vala-port bug
Comment 29 Parin Porecha 2014-05-23 12:06:06 UTC
Created attachment 277054 [details] [review]
Add Undo and Redo buttons in Headerbar

was using the wrong way to add accelerator to a widget. Removed it.

GLib.Action does not have accel support.
Is there a way we can add one to a widget directly in the UI file ?
Comment 30 Michael Catanzaro 2014-05-23 12:30:03 UTC
Review of attachment 277054 [details] [review]:

Looks good

::: data/gnome-sudoku.ui
@@ +18,3 @@
+                        <property name="can_focus">True</property>
+                        <property name="focus_on_click">False</property>
+                        <property name="action-name">app.undo</property>

You can try:

<accelerator key="Z" signal="activate" modifiers="GDK_CONTROL_MASK"/>

I think it's nicer to separate accelerators for actions from the buttons themselves, using Gtk.Application.add_accelerator(). This lets you define all your global accelerators in the same place.
Comment 31 Michael Catanzaro 2014-05-25 13:23:04 UTC
Review of attachment 277048 [details] [review]:

Good
Comment 32 Michael Catanzaro 2014-05-25 13:23:08 UTC
Review of attachment 277044 [details] [review]:

Good
Comment 33 Michael Catanzaro 2014-05-25 13:24:15 UTC
Review of attachment 277054 [details] [review]:

OK, with the accelerators coming in a separate patch