solus-cold-storage / journal

GNU General Public License v2.0
9 stars 5 forks source link

Added attempt at accels. It compiles and executes but does not work. #10

Closed MatthewScholefield closed 9 years ago

MatthewScholefield commented 9 years ago

Note: Unlike the title in the commit, this does NOT work.

I attempted doing it the way in the chess example. I have a string constant called SAVE_NAME and I create an ActionEntry array which is what I believe should make the function "save_temp()" execute when it receives the SAVE_NAME signal. Currently save_temp() prints SAVE CODE to the command prompt.

In order for this method to work, I changed EvolveWindow to inherit Gtk.ApplicationWindow which I believe functions the same way but with added commands such as win.add_action_entries() which is used for the accels. I'm not sure whether the function enable_window_action is required, but I added it just in case. The type cast in that function causes a critical warning during compilation. The commented code block is unrelated.

ikeydoherty commented 9 years ago

Have you actually added a SimpleAction to the ActionMap, i.e. the application? Also remember the difference between win. and app. actions.

ryanleesipes commented 9 years ago

I tried to implement something similar to the stuff we saw in Pantheon. Perhaps we should take a look at how Gedit does this, as the Elementary OS projects always seem convoluted to me.

ikeydoherty commented 9 years ago

I'll look after work.

ikeydoherty commented 9 years ago

Looked at your code, multiple things wrong here. I've mentioned each of these to you on IRC a few times...

EvolveWindow should have a constructor, not a method to return a new instance. EvolveWindow needs to be a GtkApplicationWindow, and you add the accel's to the GtkApplication instance. Actually add the actions to the map.

Simple example:

diff --git a/src/Journal.vala b/src/Journal.vala
index 4802d9e..2bc4cb2 100644
--- a/src/Journal.vala
+++ b/src/Journal.vala
@@ -18,7 +18,7 @@

 using GLib;

-class Application : Gtk.Application{
+class Application : Gtk.Application {

    public bool window_created;
    public EvolveJournal.EvolveWindow win;
@@ -60,8 +60,7 @@ class Application : Gtk.Application{

    public void run_application(EvolveJournal.EvolveNotebook notebook){
        if (window_created == false){
-           var win = new EvolveJournal.EvolveWindow ();
-           win.EvolveWindow(notebook, this);
+           var win = new EvolveJournal.EvolveWindow (notebook, this);
            window_created = true;

            win.delete_event.connect((win,e) => { Gtk.main_quit (); return false; });
@@ -78,4 +77,4 @@ class Application : Gtk.Application{

 static int main(string[] args){
    return new Application ().run (args);
-}
\ No newline at end of file
+}
diff --git a/src/Window.vala b/src/Window.vala
index 9dc03f3..f1d301c 100644
--- a/src/Window.vala
+++ b/src/Window.vala
@@ -22,10 +22,11 @@ namespace EvolveJournal {

   public string buffer;

-  public class EvolveWindow : Window {
+  public class EvolveWindow : Gtk.ApplicationWindow {

-    public Gtk.Window EvolveWindow (EvolveJournal.EvolveNotebook notebook, Gtk.Application application) 
+    public EvolveWindow (EvolveJournal.EvolveNotebook notebook, Gtk.Application application) 
     {
+     Object(application: application);

     this.window_position = WindowPosition.CENTER;
     set_default_size (600, 400);
@@ -67,14 +68,15 @@ namespace EvolveJournal {
         save_file(notebook);
     });

-    /*set_accels_for_action("win.EvolveWindow.save_action", {"<Control>s"});

     var action = new SimpleAction("save_action", null);
     action.activate.connect(()=> {
+      message("I R SAVING... I R SAVING... ALL ALONG... THE FILESYSTEM TREE...");
       save_file(notebook);
     });
+    application.set_accels_for_action("app.save_action", {"<Ctrl>S"});

-    application.add_action(action);*/
+    application.add_action(action);

     MenuButton menu_button = new MenuButton();
     var popover = new Popover(menu_button);
@@ -102,8 +104,6 @@ namespace EvolveJournal {
     this.add (vbox);

     headbar.show_all();
-
-    return this;
     }
   }

Now, one, the code needs cleaning up :) I have no idea what indentation of coding style you're using, so I can't make a valuable contribution by way of a pull request. If you're going to use OOP methodology, then use constructors and inheritance correctly. Note how I'm initialisation EvolveWindow via the parent GtkApplicationWindow's constructor via chaining. This initialises the parent object first, ensuring All Werks (TM)

"app." actions are application wide, or in the GtkApplication mapping. "win." actions are per window, or GtkApplicationWindow.

Second note on OOP principles and inheritance, it occurs to me your notebook logic seems off. You would create an EvolveWindow which would manage its own EvolveNotebook, not create the future child widget of a parent and pass it to them. :)

ikeydoherty commented 9 years ago

Screenshot demo: screenshot from 2015-01-22 00 43 05

ryanleesipes commented 9 years ago

Actually have been thinking about everything you mentioned. I'm going to clean it all up tomorrow. It needs to follow some rules as well, I'm going to refer to your development doc for that. Clean up will be long and arduous. But, alas, this project was destined to be messy as I've been learning the whole time while making it.

Thanks for the feedback Ikey!

On Wed, Jan 21, 2015, 6:51 PM Ikey Doherty notifications@github.com wrote:

Screenshot demo: [image: screenshot from 2015-01-22 00 43 05] https://cloud.githubusercontent.com/assets/164489/5848470/b9cc1be2-a1d0-11e4-8a36-c742cf5088c1.png

— Reply to this email directly or view it on GitHub https://github.com/evolve-os/journal/pull/10#issuecomment-70952370.

ikeydoherty commented 9 years ago

My pleasure, I know its been a baptism of fire, but honestly that's the best way of doing it :)