Revised upload page #1255
Revised upload page #1255
Conversation
jstucke
force-pushed
the
revised-upload-page
branch
from
591d58b to
04fe945
Compare
|
Nice! Can we make the right arrow trigger the list of all possible entries? |
jstucke
force-pushed
the
revised-upload-page
branch
from
a7c8f3b to
0971e71
Compare
jstucke
force-pushed
the
revised-upload-page
branch
from
0971e71 to
f26d91b
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1255 +/- ##
==========================================
- Coverage 92.23% 91.85% -0.38%
==========================================
Files 379 378 -1
Lines 23188 20920 -2268
==========================================
- Hits 21387 19216 -2171
+ Misses 1801 1704 -97 ☔ View full report in Codecov by Sentry. |
…ogging the check_for_errors method is not useful anymore, since all non-optional form fields are 'required' (meaning you cannot click on 'submit' if they are missing) and if they are missing, we get a BadRequestKeyError anyway the improved error logging for BadRequestKeyError will show which key is missing (otherwise it would fail silently)
replaced bootstrap selects with dropdowns and made styling more coherent
maringuu
force-pushed
the
revised-upload-page
branch
from
f26d91b to
efff551
Compare
There was a problem hiding this comment.
In addition to the commend below, I have two request but I am approving anyways to not get this stuck another week.
- Please make the button toggle the list. See the example code linked in the above comment.
- Please make the list scroll if it overflows.
For example, the following cs makes it show 10 items and add a scrollbar if it overflows.
.autocomplete-items {
// [...]
max-height: 1000%;
overflow-x: hidden;
overflow-y: auto;
}
| function closeAll(currentElement) { | ||
| if (currentElement && currentElement.id.endsWith("-button")) { | ||
| // if we clicked on the dropdown button we don't want to close the autocomplete list (we just opened) | ||
| currentElement = getAutocompleteElement(currentElement.parentNode.parentNode); | ||
| } else if (currentElement && currentElement.tagName.toLowerCase() === "input") { | ||
| // also don't close the autocomplete that belongs to an input we just clicked | ||
| currentElement = getAutocompleteElement(currentElement.parentNode); | ||
| } | ||
| const elements = document.getElementsByClassName("autocomplete-items"); | ||
| Array.from(elements).forEach((element) => { | ||
| if (currentElement !== element) { | ||
| element.parentNode.removeChild(element); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Why remove the elements instead of setting them to display: none like in this example?
Can we generate the list in jinja instead of javascript if we do this?
Removing the elements seems unusual to me, but more importantly is very annoying when interactively designing the css.
(I wanted to add a scrollbar to the lists if they are too large, and I had to re-select it everytime because the whole list was removed from html)
There was a problem hiding this comment.
Why remove the elements instead of setting them to display: none
I followed another guide. display: none seems to generally work, too, but sometimes it seems to get buggy (e.g. the up/down keys not working for selecting the dropdown entries), so I'm in favor of simply leaving it as it is
Can we generate the list in jinja instead of javascript if we do this?
No, because the "Device Name" list gets generated dynamically (i.e. client-side with JavaScript) after selecting device class and vendor (only device names for that particular combination are displayed)
I think I found a way to fix both issues (see 144dbb7) |
moment.jspackage (apparently unused for quite a while)requiredtags)