baudren / NoteOrganiser

Scientific Note Taking
MIT License
13 stars 6 forks source link

46 - New entry in contextmenu: preview #47

Closed egolus closed 9 years ago

egolus commented 9 years ago

this action directly loads the preview-tab for the selected notebook

landscape-bot commented 9 years ago

Code Health Code quality remained the same when pulling a29c389 on baudren:46-RightClickPreview into d499f74 on baudren:devel.

egolus commented 9 years ago

so what do I do here?

possible changes:

egolus commented 9 years ago

here is a way to check for the modifier-keys: (shift-click etc.) http://stackoverflow.com/questions/8772595/how-to-check-if-a-key-modifier-is-pressed-shift-ctrl-alt

landscape-bot commented 9 years ago

Code Health Code quality remained the same when pulling a17ed7b on baudren:46-RightClickPreview into d499f74 on baudren:devel.

baudren commented 9 years ago

Hey @egolus it looks pretty good!

One nitpick would be that, once shift clicked on the notebook to preview it, if I switch back to the editing pane, it is not the correct one loaded. It is a very simple fix, and I think it would improve it nicely. Otherwise, nice job, thank you!

egolus commented 9 years ago

so I need to set the tab in Editing to the notebook? All right, I'll look into it

landscape-bot commented 9 years ago

Code Health Repository health increased by 0.15% when pulling 537dcb5 on 46-RightClickPreview into d499f74 on devel.

egolus commented 9 years ago

ok, I'm switching the tab in the editor now. Tell me, if I'm doing something stupid with the notebook-path. I'm getting the name, build a full path from it and then have to get the basename back for calling Editing.switchNotebook

and have you seen, what i wrote in gitter about the PicButton ? I was thinking to have a baseclass PicButton and two classes PicNotebook and PicFolder that inherit from the baseclass. So the actions delete and preview would be implemented in PicNotebook.

baudren commented 9 years ago

Ok, I agree, it might become cleanier to separate the two classes. Remove the style from the argument list, and perform the paintEvent and specific actions in the daughter classes.

Could you also add a test in tests/test_frames.py, the function test_shelves, to try and shift click and see if it opened the library, then going back to editing and checking that the index of the tab is the right one? This way we cover the new preview function.

landscape-bot commented 9 years ago

Code Health Repository health increased by 0.20% when pulling cbb7e2e on 46-RightClickPreview into d499f74 on devel.

landscape-bot commented 9 years ago

Code Health Repository health increased by 0.20% when pulling 467c362 on 46-RightClickPreview into d499f74 on devel.

egolus commented 9 years ago

@baudren Do you see any problem with this? The actions are only added for notebooks. If I understand it correctly, even if you would connect something to the signals on a folder, they just wouldn't get emited, right?

(travis seems to have some problems at the moment .. this was the second job today I had to restart manually because it failed and I was sure it shouldn't)

baudren commented 9 years ago

I just implemented a delete method for folders, since it does make sense - it is very similar to the delete notebook one, but checks if the folder does not contain files, and ask confirmation if it is the case. If it is empty, the delete will be immediate.

landscape-bot commented 9 years ago

Code Health Repository health increased by 0.17% when pulling 7600d75 on 46-RightClickPreview into d499f74 on devel.

egolus commented 9 years ago

All right, this looks good

landscape-bot commented 9 years ago

Code Health Repository health increased by 0.37% when pulling f4e8a88 on 46-RightClickPreview into d499f74 on devel.