breach / thrust

Chromium-based cross-platform / cross-language application framework
MIT License
2.77k stars 121 forks source link

Expose Menu / Menu Item API #157

Closed spolu closed 10 years ago

spolu commented 10 years ago

This is a must have API and fairly low hanging fruit especially knowing that this has been done in node-webkit and atom-shell.

Atom-shell approach seems to be the best, exposing that API for each window. see: https://github.com/atom/atom-shell/blob/master/atom/browser/default_app/default_app.js https://github.com/atom/atom-shell/blob/master/docs/api/menu.md

And the implementation: https://github.com/atom/atom-shell/blob/master/atom/browser/api/atom_api_menu.h

-- Edit miketheprogrammer - Added Todos TODOS ( Roadmap )

-- Investigate better solution to events. Long Polling causes an extreme amount of CPU Wakeups with iterative loops for (N,M) where N is the number of registered bindings and M is the number of events registered to that binding. The amount of work done on idle wakeups seems negligible however, in order to maintain responsive and reduce jank, the polls have to be very short intervals, highest probably 200 ms, although 40-160ms would be the golden range. Preferably some sort of graceful throttling method would be great.

spolu commented 10 years ago

Assigned to @miketheprogrammer

miketheprogrammer commented 10 years ago

Accepted.

spolu commented 10 years ago

MISSION ACCEPTED ;-)

miketheprogrammer commented 10 years ago

@spolu I wish I had known how horrible Cocoa and Objective C were. Well maybe not horrible, but definitely not my forte. Studying up, and hacking away. Working on building out core func and OSX rendering. Then I will work on the rest once I have a good foundation on Exo.

spolu commented 10 years ago

:+1: it's worth it!

miketheprogrammer commented 10 years ago

@spolu I am finally breaking this apart successfully. chromium ui has a MenuModel, which is a reasonably abstracted interface for creating menus. Atom, uses this, and uses a MenuController as an adapter to the respective environments (i.e. Cocoa). I still cant quite figure out how Cocoa works programattically, but I have got at least some sort a layout to go with.

spolu commented 10 years ago

Awesome. Aura and osx are the two needed targets!

miketheprogrammer commented 10 years ago

@spolu ZCBenz does some magical shit in Atom man. Somehow, I havent figured out how yet. He writes his API's to use actual C arguments, and has some sort of Marshaller i think that converts the v8 args to those c arguments. And all the has to do is Mate::ObjectTemplateBuilder::SetMethod.

It is quite genious. I am a bit jealous. Hating on his use of CoffeeScript however, would have preferred he use vanilla JS

spolu commented 10 years ago

Yes his 'mate' repo is pretty cool. But we won't have to handle bindings in our case right.

I was wondering if you wanted to start coding directly on the v0.7 branch directly... Should compile by tonight...

miketheprogrammer commented 10 years ago

Yea. Ill rebase off of the v0.7 branch tonight, I am going to keep working on my current flow. I am so new to chromium, and related technologies, that I am doing alot of studying and figuring out how to architect this in the best fashion. Trying my best to imitate Atom-Shell but due to my lack of experience it is difficult to compare your current codebase with atom-shell.

Also once you get v0.7 working it will make building on linux VM so much easier, and ill be able to build for GTK.

Quick Question: (Revised) Why did you say to target Aura and OSX. Why not GTK and OSX.

spolu commented 10 years ago

In v0.7 there won't be GTK implementation anymore. Aura is a windowing system used by chrome on windows and Linux since Chrome 35!

Building will be super fast indeed! ;-)

miketheprogrammer commented 10 years ago

Ok great. Once you get that branch up ill reference it. I think for this version I am going to try to implement a simpler solution that Atom-Shell. I am going to implement a very simple api {item: "", trigger: Function }; similar to ContextMenu

miketheprogrammer commented 10 years ago

@spolu actually I dont think I can achieve as simple an api as context menu. Where triggers are managed by the js I believe. Due to the fact that a menu is a tree structure. Your context menu is fairly simple. 0 1 2 3 0 0 0 1 1 2

I am going to wait for 0.7 and see if I can implement a more complex MenuController, that uses Aura views ui::MenuModel. And does something similar to Atom. I still would like to be able to bind random JS functions to a click event.

spolu commented 10 years ago

In the ExoBrowser you just need to notify that a given menu item was clicked. Binding to js function can be done in the nodeJS library in let say node-shell. Right?

miketheprogrammer commented 10 years ago

Thats what I thought. However maybe I am just too tired to think properly but in the case of the above menu. Where menuitems can actually be submenus etcetera, I think mapping indexes may prove difficult. Not sure yet, this is my first attempt at doing cross framework menu mapping.

Unless actually thinking about it now. Each "menu" i create, can have its own handler that delegates events out and calls subitems handlers. So whenever i encounter a new "menu" I just create it in that fashion.

MenuItem1(SubMenu, "File") is clicked << UI Opens menu MenuItem1 :: itemAtIndex(0) is clicked << CallMenuItemTrigger.

No matter where a Menu is in the Tree, as long as all menu items under it are handled then we are fine.

Okay, now that I have worked through it by writing too you, which is often how i clear things up in my head heh, I see that it should be pretty straightforward.

spolu commented 10 years ago

Awesome. Everything is ready in the tree for you to event though the executable does not do anything yet. You should copy files related to menus from atom-shell browser/ui into src/browser/ui to get started. Make sure to keep the github copyright of course.

miketheprogrammer commented 10 years ago

What is the current build process for wip0.7 ? Is it still the same as the previous Exo impl

spolu commented 10 years ago

You need to run scripts/bootstrap.py to bootstrap brightray and then gyp --depth . exo_browser.gyp && make

miketheprogrammer commented 10 years ago

Bah humbug. gyp --depth . exo_browser.gyp does not seem to be creating the target.mk files I set GYP_GENERATORS="make" i even tried gyp --depth=. exo_browser.gyp

spolu commented 10 years ago

If it creates out/ you can still use ninja that's fine!

miketheprogrammer commented 10 years ago

Great. Got it building on linux using gyp and make.

miketheprogrammer commented 10 years ago

@spolu was readonly access intentional for ExoContributors team? Should I still be working on forks. Or should I be working on branches.

spolu commented 10 years ago

Yep, so that I can assign you on issues. Let's keep the current workflow!

miketheprogrammer commented 10 years ago

Back from vacation will resume work on this ASAP.

spolu commented 10 years ago

:+1:

spolu commented 10 years ago

I'm on holidays but will check my emails

miketheprogrammer commented 10 years ago

@spolu How best to handle cross dependencies between bindings. I.E. If in the ExoMenu I need to be able to access the ExoShell to get the current web contents. Currently we access instances using target Id's. So, should the arguments for a Menu call, contain a "shell_target_id" ??

Api Handler can pass itself to LocalCall and expose a public method GetTarget.

ExoShell *shell = caller->GetTarget(args->GetInteger("shell_target_id"));

spolu commented 10 years ago

Yes i Will have the same need for exoshell/exosession. I guess a target I'd is a decent way to go for now...

How is atomshell doing it? They pass the js wrapper?

miketheprogrammer commented 10 years ago

Yes, AtomShell has the nativemate wrapper which they just pass around everywhere using unwrap. Also they have exposed WebContents on a Window/NativeWindow object.

@spolu I have another question though, currently I am trying my best to mimick atom, and reuse code, while of course developing something that fits our situation. However I am currently only buiilding for Mac.

I will of course need to start building for Aura, very soon. Can you point me in any direction of where I can learn to do this. I believe Atom only uses gtk and x11. Or does it in fact use Aura?

edit** I see that Aura is referenced and included in several places in Atomshell. So I believe it is used. I also see Gtk and X11 mixed in, I am guessing its just ways to flavor the Aura. i.e. File Dialog.

spolu commented 10 years ago

I think it uses aura but with a gtk to aura library... There is very few resources on aura. If atom has the _views postfix then its aura even if it looks like gtk

miketheprogrammer commented 10 years ago

Sounds good, I have still yet to get my linux box set up. I will get it up sooner than later. At my current rate of speed, I think this branch will take a minimum of 3 weeks to complete and have test coverage. Barring of course changes to exo's structure during progress on 0.7

edit*: I do see that they use the _views postfix.

miketheprogrammer commented 10 years ago

Also, I have added Aura support to the list of items for 0.7 feel free to remove it if your not trying to target that. But of course Menu API is blocked by the need for Aura to be the default Renderer for non OSX platforms.

spolu commented 10 years ago

We should have both aura for linux/win and cocoa for OSX implementations of everything, including menu items, right?

miketheprogrammer commented 10 years ago

Yes. I am building menu items with cocoa and aura in mind. So far targeting cocoa primarily and keeping in mind that I will have to make it work for aura.

I do believe for aura we may need to be able to get all of the current windows for setting the menu on each window. This would mean being able to get all the current exo shell bindings that have been created. We can do this on the breach side if its easier, but keeping in mind that multiple processes may be able to connect to a running service of exobrowser, this would lead to network segmentation if we managed this on breach side. (This could either be a desired effect or an undesired effect.) For now the simple case is 1 breach instance :: 1 exo browser instance, but there may be instances where mutiple breach instances are talking to 1 exo browser instance.

miketheprogrammer commented 10 years ago

@spolu I have a preliminary way of accessing bindings from other bindings. I pass the handler instance to LocalCall every time it is called. There is a new method GetBinding which returns a binding at the specified index. Using static_cast we can then cast down the hierarchy to the expected subclass of ApiBinding.

spolu commented 10 years ago

That looks reasonable. Only thing is that I have a substantial PR coming up on that part. So you might have to merge that work on the new PR. Let's coordinate on IRC!

spolu commented 10 years ago

Menu API is mostly implemnted. We'll need to iterate on it but I think this can be closed for now

hachi8833 commented 8 years ago

+1