Customizable prints progress
Lubomir I. Ivanov
neolit123 at gmail.com
Fri Aug 28 12:58:38 PDT 2015
On 28 August 2015 at 20:47, Gehad Elrobey <gehadelrobey at gmail.com> wrote:
>
>
> On Sat, Aug 22, 2015 at 6:44 PM, Lubomir I. Ivanov <neolit123 at gmail.com>
> wrote:
>>
>> On 22 August 2015 at 19:20, Gehad Elrobey <gehadelrobey at gmail.com> wrote:
>> >
>> >
>> > On Sat, Aug 22, 2015 at 5:58 PM, Lubomir I. Ivanov <neolit123 at gmail.com>
>> > wrote:
>> >>
>> >> can we eventually support HTML/CSS editing of the statistics template,
>> >> for instance - adding/removing table columns such as year.avg_depth,
>> >> year.max_depth etc?
>> >> does that mean we need another Custom.html?
>> >>
>> >
>> > We need to add Custom.html template in printing_templates/statistics, So
>> > we
>> > can saved edited statistics template there, unless we support in-place
>> > editing.
>> >
>> >>
>> >> actually, this opens some questions about the Custom.html
>> >> concept...something that i have missed.
>> >>
>> >> how can the user edit, say the 6-dives template and save it from
>> >> Subsurface? it always saves to the Custom.html template and editing in
>> >> place is not possible. of course this is nice in a way, as they can't
>> >> modify the bundled templates but i think *they should* and it's their
>> >> problem if they make one of the bundled templates bad.
>> >>
>> >
>> > I though of this, my solution was that they can always export the custom
>> > template into subsurface template directory with the name they prefer
>> > and it
>> > will be automatically added to the list, Also it is possible to export
>> > the
>> > template anywhere then import it which will save it to the list.
>> >
>> >>
>> >> editing in-place is the way to go, which means that if the user edits
>> >> "Six Dives.html" and presses "save" the actual "Six Dives.html" is
>> >> saved - this removes the need for Custom.html.
>> >> what do you think? sorry for the late change request, but hopefully
>> >> that won't break a lot of things.
>> >>
>> >
>> > I think this will make the user interface more intuitive, but we will
>> > not be
>> > able to handle overwriting the bundled templates (We can still show a
>> > warning message).
>> >
>>
>> i agree, that it does become more intuitive as Custom.html is a bit
>> confusing.
>>
>> sometimes giving slightly more freedom to the user isn't that bad. if
>> they lose a bundled template they can always re-install the whole app
>> in the same folder.
>> ...and to make a copy of a bundled template they can just Export in
>> the same folder with a new name e.g. MyNewTemplate.html.
>>
>> let's do it with in-place editing and show the following warning if
>> any of the bundled templates is about to be saved by the user:
>> <Warning sign> (if possible)
>> "You are about to modify a template bundled with Subsurface. Do you
>> want to save your changes?"
>> <Save>, <Cancel>
>>
>> For non-bundled templates:
>> "Do you want to save your changes?"
>> <Save>, <Cancel>
>>
>> this should include "statistics/Default.html" and it will be nice to
>> be able to edit statistics templates as well.
>>
>
> Hello Lubomir,
>
> I have attached the patches for enabling in-place edit, as well as
> supporting to edit the default statistics template.
> Sorry a little bit late, I was busy this week.
>
hello,
thank you for finding the time!
the in-place editing works fine and the warning message is displayed
correctly when the templates texts are modified.
here are a couple of quick comments about the patches.
---------
0002:
+#define PREINSTALLED_TEMPLATE printOptions->p_template ==
"Flowlayout.html" || printOptions->p_template == "One Dive.html" \
+ || printOptions->p_template == "Six Dives.html" ||
printOptions->p_template == "Table.html" || printOptions->p_template
== "Two Dives.html"
+
since this is the only usage of the macro let's use a QStringList instead.
in TemplateEdit::saveSettings():
QStringList bundledTemplates;
bundledTemplates << "Flowlayout.html" << "One Dive.html" << ....
this makes it easier on the eyes for developers to bundle a new
template instead of adding a "printOptions->p_template == X" in the
macro.
then iterate "bundledTemplates" and break if a match is found and show
the following:
msgBox.setIcon(QMessageBox::Warning);
message = "You are about to modify a template bundled with
Subsurface.\n" + message;
^ note that i've added a new line and the old (simpler) message is now
on the second line.
setIcon() adds a warning icon for this specific message box only in this case.
also when creating the message box itself use:
QMessageBox msgBox(this);
^ "this" will set the parent and use it's little Subsurface icon for
the modal message box.
---------
0001 & 0002 & 0003:
there many are instances of braces around single lines in if/then
statements, such as:
if (printOptions->type == print_options::DIVELIST) {
TemplateLayout::writeTemplate(printOptions->p_template,
ui->plainTextEdit->toPlainText());
} else if (printOptions->type == print_options::STATISTICS) {
TemplateLayout::writeTemplate(QString::fromUtf8("statistics") +
QDir::separator() + printOptions->p_template,
ui->plainTextEdit->toPlainText());
}
please remove the braces to make it look like:
if (printOptions->type == print_options::DIVELIST)
TemplateLayout::writeTemplate(printOptions->p_template,
ui->plainTextEdit->toPlainText());
else if (printOptions->type == print_options::STATISTICS)
TemplateLayout::writeTemplate(QString::fromUtf8("statistics") +
QDir::separator() + printOptions->p_template,
ui->plainTextEdit->toPlainText());
i can see i've missed more of these in my reviews - for example in
TemplateEdit::colorSelect(), but if you'd like to change them do so in
a separate patch (as it's for code that's already in master).
BTW, at some point we need to tr() all user interface strings as those
need to be translated before the new Subsurface release. for instance:
tr("You are about to modify a template bundled with Subsurface.\n")
dialog titles...
dialog texts...
etc...
i was going to remind you about that during GSoC but i forgot...
lubomir
--
More information about the subsurface
mailing list