← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~trb143/openlp/dnd into lp:openlp

 

Review: Needs Fixing
A few things:

1) line 74: hasUrls should be hasUrls()
2) line 143: spelling (runn)
3) line 146-147, 165-166: wrong indents
4) Dropping a few files (e. g. images) makes the mainwindow process bar going from 0 to 100 (in one step) as many times as many files I drop (instead of increasing the bar after processing each file)
5) when I DnD a service file to the service manager without asking me to save an already opened service
6) I thought you were going to add support for folders as well (e. g. image library)
7) line 220-222, ...: I do not like this. All ListWidgets which allow dnd need to connect to the signal, thus it should be taken care of when activating the dnd. I am not sure if we should call/have an activate method. I was thinking about specifying this in the constructor or to have an attribute (like hasDnD) in the media manager.
8) When I DnD two files (e. g. "Copy of Foo" and "Foo") a dialog pops up and says that "Copy of Foo" is already in the list. I guess this will be/is fixed in your b1 branch?
9) The "duplicate error message" does not pop up once, instead it pop ups x times which can block the program (as you have to close x dialogs)
-- 
https://code.launchpad.net/~trb143/openlp/dnd/+merge/69526
Your team OpenLP Core is subscribed to branch lp:openlp.


References