openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

Review: Needs Fixing

I've just been browsing around your code, and I can't say I'm terribly happy.

- You've completely replaced the list widget with a tree widget. Tree widgets
  are for trees, list widgets are for lists. Please add the list widget back
  in; there are ways and means to achieve what you want without throwing out
  what we are already using.

- In using a tree widget, it leaves small artifacts next to items when in
  "list view" mode. This is ugly and erodes at one of the reasons people
  actually use OpenLP (even if they don't say so): it looks GOOD.

- Your naming conventions do not properly match our coding standards, in
  particular, "choosegroupform" and friends, which matches neither old nor
  new naming conventions.

- Folders (or groups, as you call them), need an icon. If you can't find an
  Oxygen style icon, I can give you one which I think should be used.

Please don't feel discouraged, you've done some good work here and I'm impressed, I just want to move it up to GREAT.
-- 
https://code.launchpad.net/~arjan-i/openlp/images_groups/+merge/146318
Your team OpenLP Core is subscribed to branch lp:openlp.