[WIP] Provide option to custom limit use/match #2779
Conversation
|
Those pictures show the format of custom. Please help to check, if the format is acceptable. |
There was a problem hiding this comment.
There's some inconsistent indentation and lots of repetitive style tags. Since it is still WIP I assume you're cleaning these up.
Because you've asked for feedback regarding the UI: It has obvious alignment issues, e.g. the borders of the form elements are not in line with due to the inconsistent height of the elements.
| @@ -196,4 +208,31 @@ function setupAdminNeedles() { | |||
|
|
|||
| $('#last_seen_filter').change(reloadNeedlesTable); | |||
| $('#last_match_filter').change(reloadNeedlesTable); | |||
| $('#btn_custom_last_seen').click(reloadNeedlesTable); | |||
| $('#btn_custom_last_match').click(reloadNeedlesTable); | |||
There was a problem hiding this comment.
When I remember jQuery correctly it is possible to write this as $('#last_seen_filter, #last_match_filter, #btn_custom_last_seen, #btn_custom_last_match').click(reloadNeedlesTable). The same counts for the other jQuery selectors. Note that it would also be possible to assign a class to these elements and just use that.
| } else { | ||
| lastMatch = "last_match=" + $('#last_match_filter').val(); | ||
| } | ||
| return url + "?" + lastMatch + "&" + lastSeen; |
There was a problem hiding this comment.
Don't forget to encode these variables properly, e.g. using encodeURIComponent().
| }); | ||
|
|
||
| function getCustomData(type) { | ||
| var fromTimeNum = $("#" + "text_" + type + "_from_time").val(); |
There was a problem hiding this comment.
Why not just write "#text_" instead of "#" + "text_"?
|
And regarding the controls, maybe it would be better to use something like https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/datetime-local which we already do on the API keys page. And yes, that's for a time point and not a time span but I guess it would work as well. And instead of the check boxes I'd add an option "Custom" to the select elements and show the custom selector if it is chosen. But maybe someone else in the team has a better idea or you can think yourself of something better. (I'm just saying: Don't blindly implement it exactly like I suggested and put some more thought into it.) |
Thanks for your reply.
This is a good idea. |
|
What @Martchus mentioned is also what I had in mind. Have an entry "Custom" in the pull down menu. |
Codecov Report
@@ Coverage Diff @@
## master #2779 +/- ##
==========================================
- Coverage 92.00% 91.99% -0.01%
==========================================
Files 190 190
Lines 11810 11811 +1
==========================================
Hits 10866 10866
- Misses 944 945 +1
Continue to review full report at Codecov.
|
|
will commit a new pr, so close this one |
Related: https://progress.opensuse.org/issues/53675