Manage required attribute on date range inputs #781
Conversation
taylor-steve
force-pushed
the
739-date-range
branch
3 times, most recently
from
55dfb72
to
a60f9dc
Compare
|
I'm not sure the user would know how to back out of this situation if they accidentally type into the start field and later decide not to submit a range. |
|
@taylor-steve Why not push this upstream into the blacklight-range-limit plugin rather than doing this change locally? |
|
|
||
| export default class extends Controller { | ||
| connect() { | ||
| this.beginInput = this.element.querySelector('input[name="range[date_range][begin]"]'); |
There was a problem hiding this comment.
Can you use a stimulus target here rather than doing a querySelector?
There was a problem hiding this comment.
I found this preferable to monkey-patching or subclassing RangeFormComponent to get at render_range_input to add the data target. Unless there's an easier way I'm missing?
| export default class extends Controller { | ||
| connect() { | ||
| this.beginInput = this.element.querySelector('input[name="range[date_range][begin]"]'); | ||
| this.endInput = this.element.querySelector('input[name="range[date_range][end]"]'); |
There was a problem hiding this comment.
Can you use a stimulus target here rather than doing a querySelector?
There was a problem hiding this comment.
I found this preferable to monkey-patching or subclassing RangeFormComponent to get at render_range_input to add the data target. Unless there's an easier way I'm missing?
| this.endInput.addEventListener('input', event => this.updateInputRequiredState(event)); | ||
| } | ||
|
|
||
| updateInputRequiredState(event) { |
There was a problem hiding this comment.
Why don't we just set both fields to required statically?
There was a problem hiding this comment.
I want to maintain the user's current ability to reset the date range search by blanking out both sides of the range. You're correct in that my logic could be consolidated though, thanks.
There was a problem hiding this comment.
I hadn't considered that. Can you add a comment about that use case here?
There was a problem hiding this comment.
Good idea. I added a comment and a note to check the template because it's only overridden to set the data target. I think it's likely this code gets removed as a result of the production WC.
taylor-steve
force-pushed
the
739-date-range
branch
from
a60f9dc
to
b391eb8
Compare
taylor-steve
force-pushed
the
739-date-range
branch
from
b391eb8
to
dd85d5f
Compare
Fixes #739.
Manages the required attribute for the inputs so the user is prompted when trying to submit an unbalanced range.