Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Hamster plugin to work with new plugin api. #465

Merged

Conversation

flavin
Copy link
Contributor

@flavin flavin commented Sep 23, 2020

ref #114

fixes #198
fixes #80
closes #114

@diegogangl diegogangl added enhancement plugins Plugins and extra backends labels Sep 23, 2020
@diegogangl
Copy link
Contributor

diegogangl commented Sep 26, 2020

I'm impressed this still works!

UI Ideas

  • Remove the tab bar in the preferences window. It's not needed as there's only one tab and it will likely stay that way forever
  • Remove the repetition in "use X from". Capitalize and leave the name of the thing, eg: "use activity title from" -> "Activity Title"
  • Remove the "OK" button. Settings are saved as they are changed and we already have the X button to close the window
  • This is a little more annoying to do (but it looks good): Put the settings in a GtkList. Look at GTG's preferences to see how it looks. You can probably copy the rows from gtk/data/general_preferences.ui to have a starting point.
  • Move the button in the main window headerbar to the other side, after the search button. Also use a symbolic icon instead of text. alarm-symbolic might work, but maybe you can find something better (you can find them with the gtk3-icon-browser app)
  • Move the "Start Tracking" button in the text editor to the three-dot menu.
  • Add a title to the records section (since the button won't be there to be a title). Also make the text darker so it's easier to read

Reviving

There's a few extra steps in order to make this a living plugin again.

  1. Move the hamster folder and hamster.gtg-plugin files out of the unmaintained folder
  2. Add subdir('hamster') and 'hamster.gtg-plugin' to the meson.build file in plugins
  3. Add another meson.build file in the hamster folder. You can copy the one in the sendmail plugin. You need to the three files there and rename things accordingly

Thanks for working on this!

GTG/plugins/unmaintained/hamster.gtg-plugin Outdated Show resolved Hide resolved
GTG/plugins/unmaintained/hamster/hamster.py Outdated Show resolved Hide resolved
GTG/plugins/unmaintained/hamster/hamster.py Outdated Show resolved Hide resolved
GTG/plugins/unmaintained/hamster/hamster.py Show resolved Hide resolved
GTG/plugins/unmaintained/hamster/hamster.py Show resolved Hide resolved
@flavin flavin force-pushed the 114-enable_hamster_plugin branch 2 times, most recently from 9b82ec8 to df8a3ce Compare September 27, 2020 15:42
@flavin
Copy link
Contributor Author

flavin commented Sep 27, 2020

@diegogangl thanks for your valuable comments, I just pushed a new version with your suggestions.
Now I'm using alarm-symbolic and to stop it process-stop-symbolic.
I'm not sure if put the icon in the task view too, for now is in the menu in the task view.

@flavin flavin requested a review from diegogangl September 27, 2020 15:45
@flavin flavin force-pushed the 114-enable_hamster_plugin branch from df8a3ce to c49ac4c Compare September 27, 2020 15:48
@diegogangl
Copy link
Contributor

Great work!
Good thinking on the stop icon.

One thing I noticed is that when you start a task in the editor, the icon toggling gets confused and it shows stop for start and viceversa. Something similar happens if I set the task to done with the timer on.

Ideally the timer should auto-stop if I dismiss or set the task to done.

@flavin
Copy link
Contributor Author

flavin commented Sep 28, 2020

hi @diegogangl just pushed a fix for

One thing I noticed is that when you start a task in the editor, the icon toggling gets confused and it shows stop for start and viceversa

but I can't reproduce what you mention marking as done or dismiss (when I do this with shortcut, on browser with context menu or button or in task editor ) the hasmter timer is stop to me all the time

@diegogangl
Copy link
Contributor

Here's what I mean by getting confused . I think what confuses me is that the button state depends on whether there's a task being being tracked, but the icon depends on whether the tracked task is selected. So if you select a different task in the browser the icon changes.

Screenshot from 2020-10-02 20-18-44

@diegogangl
Copy link
Contributor

Another thing: The plugin name shows up in the list with quotes.

@flavin flavin force-pushed the 114-enable_hamster_plugin branch from 581e502 to 7377aae Compare October 3, 2020 14:18
when a post is started in edit mode, and then is closed it will reload the button in current browser task selected.
@flavin flavin force-pushed the 114-enable_hamster_plugin branch from 7377aae to dc42b49 Compare October 3, 2020 14:43
@flavin
Copy link
Contributor Author

flavin commented Oct 3, 2020

@diegogangl thanks again, I found it I had dark theme and it wasn't so clear there your image make it clear the issue, now it doesn't look like that toggle,

and also remove the quotes in plugin name

@diegogangl
Copy link
Contributor

Alright, we're almost there! Only two more things I noticed:

  • When running in debug mode the plugin sometimes spams the terminal. For instance switching tags in the task browser prints a ton of messages like INFO - hamster:on_task_deleted:146 - Hamster: task deleted b047bc45-7b2e-4141-8390-3976f49e0e84
  • If you open the text editor for tasks and start tracking one, the other will have a "stop tracking button" that actually stops the previous task and start tracking this one. This can be a bit confusing, I think the best solution would be to check if the current task in the text editor is the one being tracked. In that case show the stop button, otherwise show the start button.

- store the menu in a pool for tasks to handle different states
- remove hamster plugin logs.
@flavin
Copy link
Contributor Author

flavin commented Oct 4, 2020

Hi @diegogangl for now I pushed a version removing logs, and handling the state of the menu in the editor to avoid that confusion when you open an editor (or multiple and change state in main window or different editor windows)

Do you prefer move the menu to a button with icon "alarm-symbolic" like main windows? to me sound good I can try to handle and replace the button in the menu.

@diegogangl
Copy link
Contributor

Thanks for the updates!

Hi @diegogangl for now I pushed a version removing logs, and handling the state of the menu in the editor to avoid that confusion when you open an editor (or multiple and change state in main window or different editor windows)

I'd rather not add more buttons to the editor, since it's already pretty crowded. And in the future we might include the editor as a third pane in the main window, so we might have to remove the button anyways.

I found another thing, if you quit gtg while a task is being tracked hamster keeps tracking that task. If you open gtg again, you get the option to start that task instead of stop (but it actually stops the task).
I think the easiest way to prevent this is to stop tracking when the plugin is deactivated (which also happens when you close gtg). That can be done in the deactivate method.

@flavin
Copy link
Contributor Author

flavin commented Oct 6, 2020

Hi @diegogangl thank you as always with great feedback

if you quit gtg while a task is being tracked hamster keeps tracking that task

to me doesn't sound like a problem, hamster can work in parallel and close, crash or disable plugin in GTG shouldn't affect that.

If you open gtg again, you get the option to start that task instead of stop (but it actually stops the task).

this doesn't happen to me, I click once gtg show the stop button, and don't do anything to hamster.
I was thinking about this to make it clear and maybe

  • if hamster is running a task, get it
  • then match by title, tag, and maybe even description to match hamster acitivity with GTG tasks, but I think we can't be completely sure is just a text comparation and is something can be changed in both places gtg and hamster easily.
  • so if an opened task match to active activity in hamster show the stop in gtg from start.

maybe the comparation options could be options

  • "at start match already running hamster activity with gtg task by: [title, tags, title+tags]"
  • "stop hamster when GTG task is closed"

but to me sound like a new iteration, maybe a next phase, and start merging the minimum viable product.

also for next phase apart of these options or other if appears feedback,
also maybe we can improve the plugin api to trigger events in different tabs, open, actionable, closed. since now I only see events triggered in open tasks,
or api to add context options when is clicked the secondary button of the mouse

@diegogangl
Copy link
Contributor

diegogangl commented Oct 6, 2020

this doesn't happen to me, I click once gtg show the stop button, and don't do anything to hamster.

Hm you are right, I tried tracking only one task this time and it works like that.

if hamster is running a task, get it
then match by title, tag, and maybe even description to match hamster acitivity with GTG tasks, but I think we can't be completely sure is just a text comparation and is something can be changed in both places gtg and hamster easily.
so if an opened task match to active activity in hamster show the stop in gtg from start.

It'd be interesting to check if Hamster keeps some sort of ID for activities that can be fetched. Then we could link hamster activity ids to a task id, those can't be changed by the user

but to me sound like a new iteration, maybe a next phase, and start merging the minimum viable product.

Agreed. If you don't have anything else to do with this branch I'll merge it later then.

BTW, make sure to include these in one of the commits or the PR description since you actually brought it back to life and fixed these bugs :)

fixes #198
fixes #80
closes #114

also maybe we can improve the plugin api to trigger events in different tabs, open, actionable, closed. since now I only see > events triggered in open tasks,
or api to add context options when is clicked the secondary button of the mouse

Yeah, the API is missing several utilities. I have been avoiding it since the UI is still going several big changes and refactors. For instance we will probably start moving away from GtkTreeView for 0.6. Also I wonder if it wouldn't be better to use signals instead of those callbacks. Anways, feel free to open an issue if you have some ideas.

@flavin
Copy link
Contributor Author

flavin commented Oct 7, 2020

@diegogangl added those 3 into the PR description

@diegogangl diegogangl merged commit 975ca8e into getting-things-gnome:master Oct 8, 2020
@ederag
Copy link

ederag commented Oct 17, 2020

Sounds good !
Going further, you might be interested in v3.0 new functions.
A Fact can now be checked for validity with check_fact if desired,
then added directly with add_fact.
(no need to build an intermediate string).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement plugins Plugins and extra backends
Projects
None yet
4 participants