← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~smpettit/openlp/data-path into lp:openlp

 

I've made some fast tests on Linux. In general it works. In detail there are a lot things to improove.

A general question (maybe I missed sone previous discussion): Why do we need this subfolder structure? I can understand the data directory as this allows us to place more directories into ~/.openlp/ , but why do you add the OpenLP directory into the structure? It ends up with a long directory structure without additional use. People should be able by their own th structure their directories.

The label 123-126 shouldn't be shown with this text when the copy checkbox is not ticked. Maybe make the warning bold.

206: remove the linebreak at the end of this line. In fact: I think it's best to remove the whole dialog. The fact, that copying is happening while OpenLP ends can be put on a lable like newDataDirectoryHasFilesLabel. The people are still able to revert and the directory is shown in the edit line.

Never mention the full path. People don't have to be confused with the full path and the details wich data structure do we use. It is fine for people to know the base folder they selected.

Add labels: "Current:", "New:"

Your buttons: Use toolbuttons with tooltips as you see it in the defaultImageGroupBox.

Changes should be applied in the save() method. If I press cancel in the dialog, I don't want the settings to be applied.

Check, if write access to the choosen directory is permitted.

You show lots of dialogs. Think about, which are necessary. You can maybe merge some of them. For example: Don't show two dialogs at program end, but merge them to one.

According to non-Windows systems: other OS are case sensitive. Name the "Data" directory without caps. If I'm not misleaded, then all* directories which OpenLP created from a fixed name are lower case. (* except the presentation program names). If we do not stick to a consistent naming we have to do all name comparation case insensitive.

-- 
https://code.launchpad.net/~smpettit/openlp/data-path/+merge/89612
Your team OpenLP Core is requested to review the proposed merge of lp:~smpettit/openlp/data-path into lp:openlp.


Follow ups

References