Search/filter users in the share dialog #3417
Conversation
| setTimeout(function() { | ||
| var $searchInput = this.$('.js-search-input'); | ||
| $searchInput.focus().val($searchInput.val()); | ||
| }, 0); |
There was a problem hiding this comment.
- Using timeouts are really keen to strange race conditions, what's the reason for using it here?
Btw, "this" context is actuallywindow, so the full documented is searched instead of just the scope of this view which I assume is what you want either. - Why set the value it already has to itself?
There was a problem hiding this comment.
Thanks.
- I just realized it was broken (because of the
this). It wasn't working properly without the timeout. Perhaps a race condition as you said with something on the dialog. I'll make sure it's still required. - Re-setting the value of the input again makes the cursor move to the end.
There was a problem hiding this comment.
Hmm, I still feel I lack information on why both these things are necessary to do in the first place, can you elaborate? :)
There was a problem hiding this comment.
The timeout was finally not necessary, so it's gone. Good catch!
Focussing and moving the cursor to the end of the search input let's me keep typing after submitting a search (we're actually re-rendering everything when a search is submitted, and that's the reason why need this).
There was a problem hiding this comment.
Great to hear! :)
And now I understand what you needed it for. For this kind of cases (inputs involved) I'm generally skeptical about re-rendering the whole view (which includes the input), instead it's a good motivation to extract the listing to a separate view that is re-rendered alone in the event of a search. Not saying you would have to do it if it works now, but more a remark for future stuff. :)
There was a problem hiding this comment.
Yes! I only realized about it when I was writing the previous comment. I'll keep it mind for next time! Thanks!
|
Apart from comments 👌 |
|
Frontend tests were OK 👍 (details) |
|
Frontend tests were OK 👍 (details) |
|
@viddo, I rewrote the template to reuse It doesn't look exactly as the original mockup, but I think it looks 👍
@saleiva what do you think? |
|
👍 |
|
be carerful with the be open my friend text :P @carlostallon awesome copywriting skills |
|
Changed it to:
|
|
🇯🇵 |
|
🎏 |
|
Frontend tests were OK 👍 (details) |
|
Frontend tests were OK 👍 (details) |
Search/filter users in the share dialog
|
Frontend tests were OK 👍 (details) |
|
Frontend tests were OK 👍 (details) |
Fixes #1923.
Some decisions I made along the way:
@viddo, this one is for you. I finally decided to move some stuff into view_model.js since that's the pattern we're following in this case.
share_view.jsgets, sets, and listens to changes on thesearchattribute. I will deploy to staging so that you can QA.