-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
💄 [#2741] Add multiselect listbox mobile design #1398
Conversation
df21052
to
78153f0
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1398 +/- ##
========================================
Coverage 94.59% 94.59%
========================================
Files 1063 1063
Lines 39681 39681
========================================
Hits 37537 37537
Misses 2144 2144 ☔ View full report in Codecov by Sentry. |
7bf973f
to
a34befb
Compare
7524837
to
90b41e6
Compare
90b41e6
to
30d7b27
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message for 0 results after filtering ("U heeft op dit moment nog geen lopende aanvragen.") is misleading. Not part of this PR, I know, but IMHO we should consider adding a suggetion for possible actions ("reset uw filters of kom later terug" or something like this). Leaving the final call to you, though.
} | ||
|
||
closeFilterPopup(event) { | ||
// Close on clicking outside or pressing Escape |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to distinguish the two events and have two separate handlers: closeFilterPopupByEscape
and closeFilterPopupByOutsideClick
. That way, you can (a) avoid the disjunction in your handler and (b) simplify the logic. For example, in closeFilterPopupByEscape
you can check if event.key != 'Escape': return;
and then do the removal afterwards outside the conditional (following the never-nester approach). However, I'm not too familiar with the coding conventions in JS land, so leaving it to you.
Taiga: https://taiga.maykinmedia.nl/project/open-inwoner/task/2742
Design (multiple screens and multiple States here): https://www.figma.com/design/iKGhWhstaLIlFSaND2q7cE/OIP---Designs-(new)?node-id=3196-14280&node-type=frame&m=dev