[webui] In /search, remove extra hidden form fields #4133
Conversation
There was a problem hiding this comment.
I guess that those hidden inputs are needed to send the default values, because the browser won't send any value if a checkbox is unchecked.
Is the intention of this PR to fix the label association? I haven't tried but maybe if you switch the order of the inputs it could work 🤔
|
Yes the goal is to fix the association with the check-boxes. This form is submitted via GET, and all that the default parameters in the hidden fields are 0. The search does not break when they are missing, and any remaining parameters can be handled server-side. I have not found a function of these hidden fields. |
|
@andreasstieger please prefix your commit with one of our tags |
andreasstieger
changed the title
|
@andreasstieger the commit, not the pull request :-) |
|
tag added |
|
@mdeniz please re-review |
|
The search controller is considering any value != nil as true (for these fields). So when you drop the hidden fields, you also have to adjust the controller code. Which then requires a small change in the search widget form (we have to provide the default search parameters). I also tried having the default parameters defined in the controller, but that did not work because for empty checkboxes the form does not create a parameter key/value pair. I did these changes here and did some testing. It seems that everything works like before. Though someone else should review my changes and double check. @mdeniz @hennevogel ^? Feel free to cherry pick or stash that commit into your changes:-) |
|
Hey, @andreasstieger can you add what @bgeuken says? I will review it when done 👍 |
|
@andreasstieger ping |
This also fixes the association of the labels with the checkboxes.
This will allow us to drop some hidden fields from the search form (5af7b8e). Because of that change we have to add the default search parameters to the form of the 'global-search-form'.
|
Added a6b2347. I do not know the code well enough to go deeper than this. |
Codecov Report
@@ Coverage Diff @@
## master #4133 +/- ##
=======================================
Coverage 88.11% 88.11%
=======================================
Files 352 352
Lines 18708 18708
=======================================
Hits 16485 16485
Misses 2223 2223
|
|
@bgeuken is this it? |
|
Yep, just tested it. There seems to be a bug with searches via title. But this also happens with out the applied changes. |
In /search, remove extra hidden form fields.
This also fixes the association of the labels with the checkboxes.