Skip to content

Allow to add callbacks directly to items, handle item ids internally - #4

Merged
Ouri028 merged 11 commits into
Ouri028:masterfrom
ttytm:feat/item-callbacks
Oct 7, 2023
Merged

Allow to add callbacks directly to items, handle item ids internally#4
Ouri028 merged 11 commits into
Ouri028:masterfrom
ttytm:feat/item-callbacks

Conversation

@ttytm

@ttytm ttytm commented Oct 7, 2023

Copy link
Copy Markdown
Contributor

Awesome updates @Ouri028 made it a lot cleaner 💙!

The change I thought of that I mentioned here #2 is still applicable.

The changes will allow to add on_click callbacks directly to items instead of needing to create an additional handler and match for items ids. It might make vtray even more simple and straightforward to use. It's a breaking change. Let me know your thoughts.

  • Example current:

    module main
    
    import ouri028.vtray
    
    enum MenuItems {
    	edit = 1
    	copy = 2
    	quit = 3
    }
    
    fn main() {
    	icon := $if macos {
    		'${@VMODROOT}/assets/icon.png'
    	} $else {
    		'${@VMODROOT}/assets/icon.ico'
    	}
    	mut systray := &vtray.VTrayApp{
    		identifier: 'VTray!'
    		tooltip: 'VTray Demo!'
    		icon: icon
    		items: [
    			&vtray.VTrayMenuItem{
    				id: int(MenuItems.edit)
    				text: 'Edit'
    				checkable: true
    			},
    			&vtray.VTrayMenuItem{
    				id: int(MenuItems.copy)
    				text: 'Copy'
    				disabled: true
    			},
    			&vtray.VTrayMenuItem{
    				id: int(MenuItems.quit)
    				text: 'Quit'
    			},
    		]
    	}
    	on_click := fn [systray] (mut menu_item vtray.VTrayMenuItem) {
    		println(menu_item)
    		if menu_item.id == int(MenuItems.quit) {
    			systray.destroy()
    		}
    	}
    	systray.on_click = on_click
    	systray.vtray_init()
    	systray.run()
    	systray.destroy()
    }
  • Example with the PRs changes:

    module main
    
    import ouri028.vtray
    
    fn main() {
    	icon := $if macos {
    		'${@VMODROOT}/assets/icon.png'
    	} $else {
    		'${@VMODROOT}/assets/icon.ico'
    	}
    	mut systray := &vtray.VTrayApp{
    		identifier: 'VTray!'
    		tooltip: 'VTray Demo!'
    		icon: icon
    	}
    	systray.items = [
    		&vtray.VTrayMenuItem{
    			text: 'Edit'
    			checkable: true
    		},
    		&vtray.VTrayMenuItem{
    			text: 'Copy'
    			disabled: true
    		},
    		&vtray.VTrayMenuItem{
    			text: 'Quit'
    			on_click: fn [systray] () {
    				systray.destroy()
    			}
    		},
    	]
    	systray.vtray_init()
    	systray.run()
    	systray.destroy()
    }

@Ouri028

Ouri028 commented Oct 7, 2023

Copy link
Copy Markdown
Owner

Awesome updates @Ouri028 made it a lot cleaner 💙!

The change I thought of that I mentioned here #2 is still applicable.

The changes will allow to add on_click callbacks directly to items instead of needing to create an additional handler and match for items ids. It might make vtray even more simple and straightforward to use. It's a breaking change. Let me know your thoughts.

* Example current:
  ```v
  module main
  
  import ouri028.vtray
  
  enum MenuItems {
  	edit = 1
  	copy = 2
  	quit = 3
  }
  
  fn main() {
  	icon := $if macos {
  		'${@VMODROOT}/assets/icon.png'
  	} $else {
  		'${@VMODROOT}/assets/icon.ico'
  	}
  	mut systray := &vtray.VTrayApp{
  		identifier: 'VTray!'
  		tooltip: 'VTray Demo!'
  		icon: icon
  		items: [
  			&vtray.VTrayMenuItem{
  				id: int(MenuItems.edit)
  				text: 'Edit'
  				checkable: true
  			},
  			&vtray.VTrayMenuItem{
  				id: int(MenuItems.copy)
  				text: 'Copy'
  				disabled: true
  			},
  			&vtray.VTrayMenuItem{
  				id: int(MenuItems.quit)
  				text: 'Quit'
  			},
  		]
  	}
  	on_click := fn [systray] (mut menu_item vtray.VTrayMenuItem) {
  		println(menu_item)
  		if menu_item.id == int(MenuItems.quit) {
  			systray.destroy()
  		}
  	}
  	systray.on_click = on_click
  	systray.vtray_init()
  	systray.run()
  	systray.destroy()
  }
  ```

* Example with the PRs changes:
  ```v
  module main
  
  import ouri028.vtray
  
  fn main() {
  	icon := $if macos {
  		'${@VMODROOT}/assets/icon.png'
  	} $else {
  		'${@VMODROOT}/assets/icon.ico'
  	}
  	mut systray := &vtray.VTrayApp{
  		identifier: 'VTray!'
  		tooltip: 'VTray Demo!'
  		icon: icon
  	}
  	systray.items = [
  		&vtray.VTrayMenuItem{
  			text: 'Edit'
  			checkable: true
  		},
  		&vtray.VTrayMenuItem{
  			text: 'Copy'
  			disabled: true
  		},
  		&vtray.VTrayMenuItem{
  			text: 'Quit'
  			on_click: fn [systray] () {
  				systray.destroy()
  			}
  		},
  	]
  	systray.vtray_init()
  	systray.run()
  	systray.destroy()
  }
  ```

Hey @ttytm ,

I like it! Will do some testing on my end to make sure everything is still working as intended, even though there are some breaking changes, currently VTray is in very early stages, so I am expecting some breaking changes 😆 .

I would like to have VTray as simple and easy to use as possible for the future as I can definitely see a Electron / Tauri framework coming to V in the near future with your webview library 😄 , so creating a simple and easy to use system tray is a MUST!

@ttytm

ttytm commented Oct 7, 2023

Copy link
Copy Markdown
Contributor Author

Hey thanks for the review!

Awsm! Lets build things brick by brick 👍

@ttytm

ttytm commented Oct 7, 2023

Copy link
Copy Markdown
Contributor Author

Giving it a short run there are some more awesomeizations to add. They would build on the changes so I'm gonna add it here as they might fit into a breaking merge.

The changes add an API that prevents friction points and helps to keep usage as clean possible.

E.g. usage now:

module main

import ouri028.vtray

fn main() {
	icon := $if macos {
		'${@VMODROOT}/assets/icon.png'
	} $else {
		'${@VMODROOT}/assets/icon.ico'
	}
	mut tray := vtray.create(icon, tooltip: 'VTray Demo!')
	tray.add_item(vtray.MenuItem{
		text: 'Edit'
		checkable: true
	})
	tray.add_item(vtray.MenuItem{
		text: 'Copy'
		disabled: true
	})
	tray.add_item(vtray.MenuItem{
		text: 'Quit'
		on_click: fn [tray] () {
			tray.destroy()
		}
	})
	tray.init()
	tray.run()
	tray.destroy()
}

vtray.create() only requires an icon the rest are optional params.

pub fn create(icon_path string, opts CreatOptions) &Tray {

It includes renaming of structs like VTrayMenuItem to MenuItem. Not prefixing with the lib name is the convention for V as it's already clear from usage and it would be redundant e.g. vtray.VTrayMenuItem -> vtray.MenuItem. It's also less text :D.

Gonna refine it to not need tray.init() anymore then it's pretty neat I think.

@ttytm

ttytm commented Oct 7, 2023

Copy link
Copy Markdown
Contributor Author

Okay now it might be it. Sorry about the spam 😅.

Result:

module main

import ouri028.vtray

fn main() {
	icon := $if macos {
		'${@VMODROOT}/assets/icon.png'
	} $else {
		'${@VMODROOT}/assets/icon.ico'
	}
	mut tray := vtray.create(icon, tooltip: 'VTray Demo!')
	tray.add_item('Edit', checkable: true)
	tray.add_item('Copy', disabled: true)
	tray.add_item('Quit', on_click: tray.destroy)
	tray.run()
	tray.destroy()
}

Just the definitions in the readme would need an update.

Eventually, using vdoc to generate a doc site automatically on changes and hosting it for free via github pages as V webview does instead could be nice: https://ttytm.github.io/webview/webview.html

@Ouri028

Ouri028 commented Oct 7, 2023

Copy link
Copy Markdown
Owner

Okay now it might be it. Sorry about the spam 😅.

Result:

module main

import ouri028.vtray

fn main() {
	icon := $if macos {
		'${@VMODROOT}/assets/icon.png'
	} $else {
		'${@VMODROOT}/assets/icon.ico'
	}
	mut tray := vtray.create(icon, tooltip: 'VTray Demo!')
	tray.add_item('Edit', checkable: true)
	tray.add_item('Copy', disabled: true)
	tray.add_item('Quit', on_click: tray.destroy)
	tray.run()
	tray.destroy()
}

Just the definitions in the readme would need an update.

Eventually, using vdoc to generate a doc site automatically on changes and hosting it for free via github pages as V webview does instead could be nice: https://ttytm.github.io/webview/webview.html

All good, just glad you took an interest in it 😄 .

This does look a lot cleaner compared to defining the complete struct, will test it and merge it 👍

I agree regarding the V doc, will have a look at implementing that as well as better code comments.

As a quick update as well, I have added two new functions

pub fn (v &VTrayApp) set_icon(icon string)
pub fn (v &VTrayApp) set_tooltip(tooltip string)

Which allows the end user to update the icon and tooltip in realtime without needing to restart the app.

@Ouri028

Ouri028 commented Oct 7, 2023

Copy link
Copy Markdown
Owner

I was thinking it would be great to have an option to get the menu item values, so how does this look?

	tray.add_item('Edit',
		on_click: fn [tray] () {
			if x := tray.get_item('Edit') {
				println(x.checked)
			}
		}
		checkable: true
	)
// get_item returns the menu item with the given text.
pub fn (t &Tray) get_item(item string) ?&MenuItem {
	for menu_item in t.items {
		if menu_item.text == item {
			return menu_item
		}
	}
	return none
}

We also add the MenuItem to the heap and make the items public.

[heap]
struct MenuItem {
pub:
	id        int
	text      string
	checked   bool
	checkable bool
	disabled  bool
}

@Ouri028
Ouri028 merged commit 47c0497 into Ouri028:master Oct 7, 2023
@ttytm

ttytm commented Oct 7, 2023

Copy link
Copy Markdown
Contributor Author

I was thinking it would be great to have an option to get the menu item values, so how does this look?

	tray.add_item('Edit',
		on_click: fn [tray] () {
			if x := tray.get_item('Edit') {
				println(x.checked)
			}
		}
		checkable: true
	)
// get_item returns the menu item with the given text.
pub fn (t &Tray) get_item(item string) ?&MenuItem {
	for menu_item in t.items {
		if menu_item.text == item {
			return menu_item
		}
	}
	return none
}

We also add the MenuItem to the heap and make the items public.

[heap]
struct MenuItem {
pub:
	id        int
	text      string
	checked   bool
	checkable bool
	disabled  bool
}

Yep that'll be useful.

A short version of the loop above would be the snip below.
Declaring as heap is not needed then as well.

// get_item returns the menu item with the given text.
pub fn (t &Tray) get_item(item string) ?&MenuItem {
	return t.items.filter(it.text == item)[0] or { return none }
}

Edit: Sorry, yours is more efficient. My mistake.
But the map approach below might be the way, it would work without loops and attribute.

@ttytm

ttytm commented Oct 7, 2023

Copy link
Copy Markdown
Contributor Author

It maybe even better to store the items in a map then. I'll just send the lib.v without further refinements for copy pasting, probably easiest to see the diffs in your editor and work with it then:

lib.v
module vtray

// Tray is the main struct that represents the tray app.
[heap; noinit]
pub struct Tray {
mut:
	instance   &VTray = unsafe { nil }
	icon       string
	identifier string
	tooltip    string
	items      map[string]&MenuItem
	callbacks  map[int]fn ()
	last_id    int = 1
}

[params]
pub struct CreatOptions {
	identifier string = 'VTray'
	tooltip    string
}

// MenuItem is a menu item that can be added to the tray.
[params]
pub struct MenuItemOptions {
	checked   bool
	checkable bool
	disabled  bool
	on_click  ?fn ()
}

// create creates the tray.
// On macOS, the tray icon size must be 22x22 pixels to be rendered correctly.
pub fn create(icon_path string, opts CreatOptions) &Tray {
	return &Tray{
		icon: icon_path
		identifier: opts.identifier
		tooltip: opts.tooltip
	}
}

// add_item adds an item to the tray.
pub fn (mut t Tray) add_item(text string, opts MenuItemOptions) {
	id := t.last_id++
	t.items[text] = &MenuItem{
		id: id
		text: text
		checked: opts.checked
		checkable: opts.checkable
		disabled: opts.disabled
	}
	if cb := opts.on_click {
		t.callbacks[id] = cb
	}
}

// get_item returns the menu item with the given text.
pub fn (t &Tray) get_item(item string) ?&MenuItem {
	return t.items[item] or { return none }
}

// run runs the tray app.
pub fn (mut t Tray) run() {
	t.instance = C.vtray_init(&VTrayParams{
		identifier: t.identifier
		tooltip: t.tooltip
		icon: t.icon
		on_click: fn [t] (menu_item &MenuItem) {
			if cb := t.callbacks[menu_item.id] {
				cb()
			}
		}
	}, usize(t.items.len), t.items.values().data)
	C.vtray_run(t.instance)
}

// destroy destroys the tray app and frees allocated memory.
pub fn (t &Tray) destroy() {
	C.vtray_exit(t.instance)
}

@Ouri028

Ouri028 commented Oct 7, 2023

Copy link
Copy Markdown
Owner

It maybe even better to store the items in a map then. I'll just send the lib.v without further refinements for copy pasting, probably easiest to see the diffs in your editor and work with it then:
lib.v

module vtray

// Tray is the main struct that represents the tray app.
[heap; noinit]
pub struct Tray {
mut:
	instance   &VTray = unsafe { nil }
	icon       string
	identifier string
	tooltip    string
	items      map[string]&MenuItem
	callbacks  map[int]fn ()
	last_id    int = 1
}

[params]
pub struct CreatOptions {
	identifier string = 'VTray'
	tooltip    string
}

// MenuItem is a menu item that can be added to the tray.
[params]
pub struct MenuItemOptions {
	checked   bool
	checkable bool
	disabled  bool
	on_click  ?fn ()
}

// create creates the tray.
// On macOS, the tray icon size must be 22x22 pixels to be rendered correctly.
pub fn create(icon_path string, opts CreatOptions) &Tray {
	return &Tray{
		icon: icon_path
		identifier: opts.identifier
		tooltip: opts.tooltip
	}
}

// add_item adds an item to the tray.
pub fn (mut t Tray) add_item(text string, opts MenuItemOptions) {
	id := t.last_id++
	t.items[text] = &MenuItem{
		id: id
		text: text
		checked: opts.checked
		checkable: opts.checkable
		disabled: opts.disabled
	}
	if cb := opts.on_click {
		t.callbacks[id] = cb
	}
}

// get_item returns the menu item with the given text.
pub fn (t &Tray) get_item(item string) ?&MenuItem {
	return t.items[item] or { return none }
}

// run runs the tray app.
pub fn (mut t Tray) run() {
	t.instance = C.vtray_init(&VTrayParams{
		identifier: t.identifier
		tooltip: t.tooltip
		icon: t.icon
		on_click: fn [t] (menu_item &MenuItem) {
			if cb := t.callbacks[menu_item.id] {
				cb()
			}
		}
	}, usize(t.items.len), t.items.values().data)
	C.vtray_run(t.instance)
}

// destroy destroys the tray app and frees allocated memory.
pub fn (t &Tray) destroy() {
	C.vtray_exit(t.instance)
}

Yeah. This will make it more easier to work with and we avoid unnecessary looping.

Thanks! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants