https://redmine.pfsense.org/https://redmine.pfsense.org/favicon.ico?16780521162016-01-22T07:15:05ZpfSense bugtrackerpfSense - Bug #5794: Apply button can go missing when the UI is in a non-English languagehttps://redmine.pfsense.org/issues/5794?journal_id=242932016-01-22T07:15:05ZAnonymous
<ul></ul><p>Phil,</p>
<p>I don't like the "automatic Apply" thing in print_info_box_np at all. When I started work on this project I would sometimes find myself editing text strings and replacing "apply" with "make" or "add" or something, just to get rid of an unwanted "Apply" button.</p>
<p>I would prefer to see the button created explicitly. Perhaps something like this:<br /><pre>
// Make an info box with an "Apply" button
print_info_box_np(gettext("Time to apply the changes we need!"), "success");
// Make an info box with a custom button
print_info_box_np(gettext("Time to apply the changes we need!"), "success", gettext("Erase"), "erase", "yes");
// Make an info box with no button
print_info_box_np(gettext("IP addresses should never contain asterisks!"), "danger", false);
print_info_box_np(gettext($displaystring, class="alert", $btnlabel=gettext("Apply"), $btnname="apply", $btnval="") {
if ($btnlabel) {
. . .
}
. . .
}
</pre></p>
<p>By defaulting to gettext("Apply"), we would maintain some sort of backwards compatibility perhaps?</p>
<p>It would be a pretty pervasive change though.</p> pfSense - Bug #5794: Apply button can go missing when the UI is in a non-English languagehttps://redmine.pfsense.org/issues/5794?journal_id=242952016-01-22T07:39:16ZAnonymous
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Feedback</i></li></ul><p>Problem alleviated by PR <a class="external" href="https://redmine.pfsense.org/issues/5794">https://redmine.pfsense.org/issues/5794</a></p> pfSense - Bug #5794: Apply button can go missing when the UI is in a non-English languagehttps://redmine.pfsense.org/issues/5794?journal_id=242972016-01-22T08:56:13ZPhillip Davisphil@jankaritech.com
<ul></ul><p>I can modify print_info_box() so it takes:</p>
<pre>
function print_info_box($msg, $class="alert-warning", $btntoshow = "close") {
switch ($btntoshow) {
case "close":
$showclose = true;
$showapply = false;
$name = "";
break;
case "apply":
$showclose = false;
$showapply = true;
$name = "apply";
break;
default:
$showclose = false;
$showapply = false;
$name = "";
break;
}
print_info_box_np($msg, $name, null, $showapply, $class, $showclose);
}
</pre>
<p>That lets you give the message text, severity level (info, success, warning, danger) and which button you want to show (if any). It massages the input parameters to make the equivalent call to print_info_box_np. The default values end up the same as previously, so callers that used the defaults do not need to change.</p>
<p>Then it is easy to modify any existing print_info_box() or print_info_box_np() call to use the new print_info_box() parameters.</p>
<p>I think I can then get rid of all calls to print_info_box_np() - after that happens then we can sort out what is really wanted in the underlying routines and perhaps make a single simpler print_info_box() that has bunches of code from print_info_box_np() ...</p>
<p>I will have a little play with a few example calls and see if it seems to work.</p> pfSense - Bug #5794: Apply button can go missing when the UI is in a non-English languagehttps://redmine.pfsense.org/issues/5794?journal_id=242992016-01-22T11:20:11ZPhillip Davisphil@jankaritech.com
<ul><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>Applied in changeset <a class="changeset" title="Fix #5794 remove print_info_box_np chackes for gettext("apply") 1) Get rid of the stristr() chec..." href="https://redmine.pfsense.org/projects/pfsense/repository/2/revisions/3b3a95e5511ef5f60f8142480ddf5adcaed88cb1">3b3a95e5511ef5f60f8142480ddf5adcaed88cb1</a>.</p> pfSense - Bug #5794: Apply button can go missing when the UI is in a non-English languagehttps://redmine.pfsense.org/issues/5794?journal_id=243032016-01-22T11:36:05ZPhillip Davisphil@jankaritech.com
<ul></ul><p>Sleep time for me now!<br />But if someone wants to re-engineer print_info_box_np() to have more nicely laid-out parameters... and change print_info_box() and the few (only 1 maybe) remaining direct calls to print_info_box_np(),<br />or even work out how to get rid of print_info_box_np(), put the code nicely in print_info_box() (probably needing an optional parameter(s) on the end),<br />then go for it.</p>
<p>It should no longer have far-reaching implications - because 99% of real code now calls print_info_box() or print_apply_box().</p> pfSense - Bug #5794: Apply button can go missing when the UI is in a non-English languagehttps://redmine.pfsense.org/issues/5794?journal_id=243652016-01-23T20:29:33ZPhillip Davisphil@jankaritech.com
<ul></ul><p><a class="external" href="https://github.com/pfsense/pfsense/commit/fd4803d2c1c778dc00bcb5b8338b29d6cb04efd8">https://github.com/pfsense/pfsense/commit/fd4803d2c1c778dc00bcb5b8338b29d6cb04efd8</a> got rid of another remaining call to print_info_box_np()</p> pfSense - Bug #5794: Apply button can go missing when the UI is in a non-English languagehttps://redmine.pfsense.org/issues/5794?journal_id=243662016-01-23T20:31:52ZPhillip Davisphil@jankaritech.com
<ul></ul><p><a class="external" href="https://github.com/pfsense/pfsense/commit/0d711d380b719219a802b2b67db4ccce6cb645d3">https://github.com/pfsense/pfsense/commit/0d711d380b719219a802b2b67db4ccce6cb645d3</a> removes print_info_box_np() completely and finishes the implementation of the re-engineered code in print_info_box().</p>
<p>All dependency on various text strings in the message is now removed. This should be happily language-agnostic and working.</p>
<p>Please test and report any problems.</p> pfSense - Bug #5794: Apply button can go missing when the UI is in a non-English languagehttps://redmine.pfsense.org/issues/5794?journal_id=243912016-01-25T09:53:44ZAnonymous
<ul><li><strong>Status</strong> changed from <i>Feedback</i> to <i>Resolved</i></li></ul>