(Cosmetic) firewall_rules page row highlighting



  • Navigate to this page:

    /firewall_rules.php?if=lan

    The anti-lockout rule line is grey, but so is the first rule. I currently only have the default entries (anti-lockout, lan, lan v6).

    Here are some things I noticed:
    Hover over a line and the entire line becomes grey.
    Click on a line, it becomes blue.
    Click on the same line, it becomes white except the actions section.
    However over a line and, if ever selected, the action section is the only part that changes color.
    LAN rule actions section always stays grey after select/deselect.

    Basically, what I saw is the the element has no class or style attribute, but then on select / deselect it is given a specific style for background. That is overriding the hover event, but not consistently in all areas of the line.

    If that's too confusing, it may have been too long of a day for me. Just ask for more details if it isn't obvious and I'll try to take some screenshots or describe it better.


  • Developer Netgate

    I know what you mean. I'll take a look at it tomorrow.


  • Developer Netgate

    Just pushed a change which I believe takes care of this. Converted the javasript to jQuery and was then able to restore correct striping after a de-select.



  • Cool. Most of it is fixed now. I still see a few things that appear odd to me. Let me know if any are intentional, though.

    1. The Anti-Lockout line is in its own 'tbody' element. That means it will always have a grey background due to the 'table-striped' class (it's the first element). The other lines are in a separate 'tbody' element which means that the first one will always start grey (matching the line above it).

    2. The select/unselect code is still directly setting the background color on the 'tr' element. That means that things still can get inconsistent as you select rows. However, the entire row is consistent now, so that's fixed.

    To further see what I mean, you can go to the page, and select the 3rd row. It will start as white, then go blue when selected, then go grey when unselected. I would expect it to return to white. I haven't checked any jquery/js code around any of this, but I would think the best thing would be to simply add a class that overrides background color when selected. Then remove that class when unselected and the existing table-striped class should resume priority for background color.

    1. the grey of the table striping does not match the grey after something is selected and then unselected.

    And finally, if the only reason for the second tbody is the jquery ui-sorting, then it shouldn't be needed. With jQuery sorting, you can pass in a selector that excludes matched items from sorting. So the 'tr' with the id of 'antilockout' could be excluded from the sorting. You also want to exclude it from being a drop target. See here for an example: https://jqueryui.com/sortable/#items


  • Developer Netgate

    The rules at the top of the table (Bogons, anti-lockout etc) are in a separate tbody because they are not sortable.

    Given that this will cause striping issues depending if there is an odd or even number or rows, I think we should remove striping from the first tbody in the table.

    So far I have not been able to reproduce the other issues you bring up, but I am still trying. I set the gray background (on de-selecting) by adding the "active" class, which I think is the correct Bootstrap way of doing the stripes.

    The grays look the same to me, but all monitors differ. I'll check with a color picker in a minute.


  • Developer Netgate

    I have removed the table-striped class and added a javascript function to apply the "active" class to alternate rows manually.

    This function is called on page load, on row de-select, and on reordering any row so that correct striping is maintained at all times.

    Seems to work more reliably now.

    I have also change the pencil/"Edit" icons in the anti-lockout/bogons etc section to a cog/Settings" since the link does not edit hte rule, but access the settings used to turn it on or off.


Log in to reply