Save info on used box-drawer in the configuration file
Cross-platform app for displaying and navigating events on a timeline.
Brought to you by:
rickardlindberg,
rogerlindberg
Suggested solution: Save the display_name of the used box-drawer in the configuration file, and select that drawer at startup of Timeline.
Involved objects:
I'll take it.
Done!
https://sourceforge.net/u/linostar/thetimelineproj-main/ci/2ffc201c86479b6d910581a07bc2094be8a9bc00/
The tests don't pass.
Please fix the test failures. Then we can merge the patch.
I investigated the problem. It seems the patch is OK itself, but there are 2 test specs that are loading only the default drawer plugin, and not the gradient plugin (loading both is now required due to the new patch). In addition, one of the test specs is using its own Config class and not loading the config from config/dotfile.py.
I modified the two test specs in a new patch, and all the tests are now passing:
https://sourceforge.net/u/linostar/thetimelineproj-main/ci/e3089a1d828920c956ef6fe0c3932c4fad9ee28b/
Last edit: Linostar 2015-02-22
Great. I merged your changes.
Sometimes that happens: Existing tests fail even though the new code is not doing anything wrong. It's better if tests are written in such a way that they only fail if the function under test fails. That is sometimes hard, so it's a good practice to always run tests before commit.
Can I also ask you to always base your commits on tip. That way I don't have to merge all the time :-)
Sure. I forgot to check there are no new changesets before I push.
Do you have a particular backlog issue that you recommend I work on next?
Last edit: Linostar 2015-02-22
Great.
I would go for one of the crash reports.
I would like a more generic solution to this issue.
Say for instance that we add a third box drawer that can be selected, the current solution can't handle that situation.
Instead of saving True or False in the config file i would suggest that the name of the drawer is saved instead like EVENT_BOX_DRAWER="Gradient Event box drawer". The name of the drawer can be retrieved with the plugin.get_display_name() method. The event box drawer can be found by the PluginFactory (globally instantiaded as factory. bad name I guess it should be changed) if we add a method like get_plugin_by_service_and_name(EVENTBOX_DRAWER, config.get_event_box_drawer). If no plugin is found the default drawer should be used.
@roger
That indeed had occured to me after I finished the patch: what if there are more than those 2 plugins in the future? But since I wasn't sure if that can happen or not, I left it at that. I'll make it more generic as you described, although new import commands will still be needed whenever a new plugin is added.
By the way, I was wondering if it is possible to make the plugin menuitems in the View menu act like radiobuttons, like if you click on one of the plugin a checkmark (or something similar) will appear next to it, and the previously selected plugin will be de-checked. I am not sure if such thing exists in wxWidgets.
I don't Think import statements are needed since all plugins are instantiated and stored in the factory.
If you want to investigate the possibilities with wxWidgets I suggest you download the wxPython demos.
All tests passed:
https://sourceforge.net/u/linostar/thetimelineproj-main/ci/3fdd9400d51278e4eb173aba5d93e5f7403cba3a/
The patch is now pushed to repo, but I still think it's not generic enough.
I created a method on the plugin factory with which we can retrieve a named plugin. You should be able to use this method in drawingarea.get_saved_drawer like this:
return self.plugin_factory.get_plugin(EVENTBOX_DRAWER, self.config.selected_event_box_drawer) or DefaultEventBoxDrawer().
In mainframe.py I would like to get rid of the ID:s used for checking selection.
I suggest we define two constants CHECKED_RB = 2 and UNCHECKED_RB = 3 and use these in the menu item definition tuple and adjust the menu creator function to check the items when they are created. CHECKED_RB will be used when self.config.selected_event_box_drawer == plugin.display_name(). When the menu items are created we can use
item.Check(checkbox == CHECKED_RB)
The ultimate test is to clone one of the event box drawers, so we get a third drawer in the plugins directory, and it should work without any other code changes.
@roger
That's great.
Concerning your approach for radiobutton checking, I understand your goal, but I don't get how we can implement it that way. If I understood you correctly, you want to get rid of the ID_PLUGINS list, and re-use wx.ID_ANY as IDs for the radiobutton menuitems. However, in order to use item.Check (and apply either CHECKED_RB or UNCHECKED_RB), we'll need to know the ID of the item so we can fetch it using FindItemById() method. Obviously, I am missing something here, or misunderstood it.
On the other hand, another approach is to use get_plugins() in pluginfactory to get the number of existing eventdrawer plugins, and use that number to create a list of the necessary IDs of the menuitems.
Last edit: Linostar 2015-02-25
I was thinking about something like the following:
for plugin in factory.get_plugins(EVENTBOX_DRAWER):
if plugin.display_name() == self.config.selected_event_box_drawer:
items.append((wx.ID_ANY, create_click_handler(plugin), plugin.display_name(), CHECKED_RB))
else:
items.append((wx.ID_ANY, create_click_handler(plugin), plugin.display_name(), UNCHECKED_RB))
And then modify the menu creator to recognize and use the values CHECKED_RB and UNCHECKED_RB to set the state of the radiobuttons.
I understand now.
I've also made changes to couple of the tests so they don't need to import drawer plugins individually:
http://sourceforge.net/u/linostar/thetimelineproj-main/ci/5a8d362586f3c1d618450203f5808d915d564321/
Now it works properly. Nice job! So I close this backlog.
Pushed to repo.