OPNsense Forum
English Forums => Development and Code Review => Topic started by: Stilez on November 12, 2019, 02:31:13 am
-
In layout_partials/base_dialog.volt (https://github.com/opnsense/core/blob/master/src/opnsense/mvc/app/views/layout_partials/base_dialog.volt), at lines 71-73 and again 104-106, there's a snippet I don't fully understand the implications/intent of. So I'm not sure how best to approach its issues either:
<table class="table table-striped table-condensed">
<colgroup>
<col class="col-md-3"/>
<col class="col-md-{{ 12-3-msgzone_width|default(5) }}"/>
<col class="col-md-{{ msgzone_width|default(5) }}"/>
</colgroup>...
The function and part of the intent is clear. At the top of the generated form HTML, it divides the standard bootstrap 12 cols into 3 "blocks" comprising { (3) + (9-msgzone_width) + (msgzone_width) } bootstrap columns respectively. The default of 5 matches the 3-4-5 default values hard coded into base_form.volt. The param is rarely used; clearly from usage and code inspection, the left "Block" is the label for controls, the "message zone" is the right hand column which seems unused, the form controls and their help text fit into the middle "block". That much is clear.
What this does however is, give the popup dialogs created from this template, a huge white space margin, and forces the help text to wrap much sooner than it needs to. Attached is a screenshot of how it renders for me. Huge white space, no centring, gets worse as the window size varies. Text wraps poorly making the form long.
I'm looking at this and wondering, can something be done to improve this, or is there a functional reason it's set that way? Of course the default can be altered for specific forms, but I'm wondering if it's actuisally got poor choices of zsizing and things. But I don't fully understand the use of the 3rd column and the overall implications and how it fits into OPNsense UI, or the function of col class=.... in this snippet, to know what to make of that. Is this axctually doing what it's intended to, and could it do it better?
Insights or suggestions how to work with this (or even minimally modify it), so that dialogs can use a little more of the dialog horizontal whitespace without unforeseen layout issues elsewhere, really appreciated!
-
Originally it came from https://github.com/opnsense/core/commit/4c736c65060c926ecc9eb7539b93454559e9d2d4 intended to display "wide" forms, see "Administration -> Intrusion detection -> Rule" for its usage.
It's not at the top of my priority list, but I wouldn't mind removing the parameter if both form types still look decent. As far as I see, we only use it in IDS at the moment, so it should be easy to refactor and test.
-
Originally it came from https://github.com/opnsense/core/commit/4c736c65060c926ecc9eb7539b93454559e9d2d4 (https://github.com/opnsense/core/commit/4c736c65060c926ecc9eb7539b93454559e9d2d4) intended to display "wide" forms, see "Administration -> Intrusion detection -> Rule" for its usage.
It's not at the top of my priority list, but I wouldn't mind removing the parameter if both form types still look decent. As far as I see, we only use it in IDS at the moment, so it should be easy to refactor and test.
Excellence of replying!
The 3 column dialog definition (whether 3:4:5 or 3:8:1) is used everywhere, so I'm hesitant to totally remove it and go back to 2 columns (3-9), because of the number of affected forms that would needed <td></td> or similar removed, and colspans updated.
However, changing the split from 3:{9-msgzone_width}:{msgzone_width} to a static 3:9:0 or a default of msgzone_width=0 with a comment explanation in the 2 volt base files, works just as well. Looks good,no bugs or glitches. Help text stretches, message boxes stay sensible widths. All nice. Also easier reversible. Tested on IDS rules dialog, as well as on dialogs with long wrapping help texts. Does that work for you? Would need minor css edit for col-md-0, to check it doesn't create a zero width box but an extra 2 margins worth of white space, but that's trivial. What do you think? Also why is this just col-md, and not other sizes? Does it need to be?
Allso self-note, if it changes or goes, the hard-coded defaults in base_form.volt also would need to change too.
-
PR submitted: https://github.com/opnsense/core/pull/3814 (https://github.com/opnsense/core/pull/3814)