fix: put unassigned review assignment contols on one small line#3780
Conversation
|
Be great if Lars can review this short one as it's all about tweaking how the form uses bootstrap5 |
Codecov Report
@@ Coverage Diff @@
## feat/bs5 #3780 +/- ##
============================================
- Coverage 87.97% 87.97% -0.01%
============================================
Files 296 296
Lines 38795 38795
============================================
- Hits 34130 34129 -1
- Misses 4665 4666 +1
Continue to review full report at Codecov.
|
jennifer-richards
left a comment
jennifer-richards
left a comment
There was a problem hiding this comment.
Looks good to me. One minor tweak suggested inline.
The "Close" button opens a similar set of fields. Should those receive the same single-row treatment? (Don't recall whether they were single row in the bs3 implementation.)
| {% endif %} | ||
| {% bootstrap_field r.form.reviewer layout="horizontal" size="sm" %} | ||
| {% bootstrap_field r.form.add_skip layout="horizontal" size="sm" %} | ||
| {% bootstrap_field r.form.reviewer layout="horizontal" wrapper_class="col row" size="sm" %} |
There was a problem hiding this comment.
I believe the row in the wrapper_class is redundant with the layout="horizontal".
There was a problem hiding this comment.
It isn't, though I agree it should perhaps have worked, but there are (I'm guessing) interactions with the fluid form classes (row and col) that defeat what layout='horizontal' does by itself.
To be clear - layout='horizontal' was already present, and was insufficient, even after adding "col".
There was a problem hiding this comment.
Also - the 'close' form is one line on main, but I think having it take more than one line makes sense - it's not used quite as often, and it makes more sense for the reason field to take some space.
There was a problem hiding this comment.
With the code as committed to the branch, the rendered output looks like:
<div class="reviewer-controls mx-1 flex-fill row">
<div class="row col">
<label class="col-md-2 fw-bold col-form-label" for="id_rtelechat-draft-ietf-httpapi-linkset-reviewer">Assign reviewer</label>
<div class="col-md-10">
<select name="rtelechat-draft-ietf-httpapi-linkset-reviewer" class="form-select form-select-sm" id="id_rtelechat-draft-ietf-httpapi-linkset-reviewer">
Note the "row col" class on the second <div>. If I drop the "row" from the wrapper_class in the template, I get:
<div class="reviewer-controls mx-1 flex-fill row">
<div class="row col">
<label class="col-md-2 fw-bold col-form-label" for="id_rtelechat-draft-ietf-httpapi-linkset-reviewer">Assign reviewer</label>
<div class="col-md-10">
<select name="rtelechat-draft-ietf-httpapi-linkset-reviewer" class="form-select form-select-sm" id="id_rtelechat-draft-ietf-httpapi-linkset-reviewer">
There's still a "row col". I diffed the page sources and only the CSRF token had changed. If I add a fake class in place of the row I see it in the output, so I'm pretty sure my change is being picked up.
Am I doing something wrong?
2 participants
No description provided.