-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Add confirmation button to share input #7196
Conversation
Sample screenshots? |
After – with confirm button icon: As said, needs JS help to work @nickvergessen @MorrisJobke. |
@danxuliu @nextcloud/javascript Anybody up to make this happen for 13? Otherwise we need to move this to 14. |
(Just assumed this was requested by a customer too? @oparoz?) |
@danxuliu has already developed the code, so this should be merged in 13 |
@danxuliu Could you show me the code? Because I would like to have this in the beta 2 then. |
As this is not yet ready as discussed with @danxuliu, I will move this to 14 and we can look into how (if at all) can backport this to stable13 later. |
So it is time to shine then I guess! |
Ready for review! Now a share is added when clicking on the confirm button of the input field (or when pressing In this pull request the order of suggestions in the autocomplete dropdown has been slightly modified. Until now the order was exact users, partial users, exact groups, partial groups, exact...; now that order became exact users, exact groups, exact..., partial users, partial groups, partial..., which in my opinion is better as all the exact matches are now shown at the beginning. Besides that some related unit tests were also improved, as well as certain behaviours (like no longer failing silently when getting the suggestions succeeded but failure content was returned (for example, if @nextcloud/designers |
@@ -22,18 +22,14 @@ | |||
'<div class="oneline">' + | |||
' <input id="shareWith-{{cid}}" class="shareWithField" type="text" placeholder="{{sharePlaceholder}}" />' + | |||
' <span class="shareWithLoading icon-loading-small hidden"></span>'+ | |||
'{{{shareInfo}}}' + | |||
' <span class="shareWithConfirm icon icon-confirm"></span>' + |
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.
Please use our latest standard: input + input[type='submit'].icon-confirm :)
You will be able to remove the custom css as well :)
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.
@skjnldsv I have been trying to use that, but... it does not work as expected (it shows some accessibility text instead of just the icon, and the position and size is not right either). Could you please take a look? :-)
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.
Of course! :)
You need to define an empty value to override html default behaviour :)
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.
Okay, did some tests, but the tooltip errors are inserting themselves between the input and the icon and break the flow. Though it's only when an error is displayed, so we could hide the submit button anyway since you can't submit with an error! Let me push!
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.
Nah, too much edits to do! Leave it like you did! We'll work on it when we will standardise the sidebar! :)
The confirmation button right now is just an icon; its behaviour will be added in the following commits. Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
As the server response is faked the search term is ignored in the tests. However, it is clearer to use a search term that would make the server return what the faked response contains. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Stubs should be restored outside the test method in which they are used to ensure that they are properly restored no matter the result of the test (for example, if an exception is thrown). Besides that, this will make possible to reuse the stub in other sibling tests without having to explicitly setup it in them. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
"OC.Notification.hide" expects the notification to be hidden to be passed as an argument. As it was being used to show a temporary notification the combination of "OC.Notification.show" and "OC.Notification.hide" was replaced by a single call to "OC.Notification.showTemporary". The timeout could have been specified in the options of the call, but it was left to the default value (7 seconds) for consistency with other notifications. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Instead of silently failing now an error is shown to the user when the ajax call to get the suggestions succeeds yet it returns failure content (for example, if an "OCSBadRequestException" was thrown in the server). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
"_getSuggestions" returned all the suggestions from the server, which are composed by exact matches and partial matches. Now the exact matches are also returned on their own parameter. This will be used by the button to confirm a share. Note that until now the order of the suggestions was "exact users, partial users, exact groups, partial groups, exact..."; this commit also changes that order to become "exact users, exact groups, exact..., partial users, partial groups, partial...". This is not a problem, as the suggestions were used in the autocomplete dropdown, and this new order is arguably better than the old one, as all exact matches appear now at the beginning. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Clicking on the confirm button now adds a share, but only if there is just a single exact match. If there are no exact matches or there is more than one exact match no share is added, and the autocomplete dropdown is shown again with all the suggestions. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Besides confirming a share by clicking on the confirm button now it is possible to do it by pressing enter on the input field. Clicking on the confirm button implicitly hides the autocomplete dropdown. On the other hand, pressing enter on the input field does not, so the autocompletion must be disabled and closed when the confirmation begins and then enabled again once it finishes. Otherwise the autocomplete dropdown would be visible and it would be possible to interact with it while the share is being confirmed. The order in which the input field and the autompletion are disabled is important. Internally, the autocompletion sets a timeout when the input field is modified that requests the suggestions to the server and then shows them in the dropdown. That timeout is not cancelled when the autocompletion is disabled, but when the input field loses its focus and the autocompletion is not disabled. Therefore, the input field has to be disabled (which causes it to lose the focus) before the autocompletion is disabled. Otherwise it could happen that while a share is being confirmed the timeout ends, so an autocompletion request is sent and then, once the share is successfully confirmed and thus the autocompletion is enabled again, the request is received and the autocomplete dropdown is shown with the old suggestions. Strange, but possible nevertheless ;-) Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
208be79
to
7b75530
Compare
@danxuliu I'm always impressed by your dedication to tests - I simply ❤️ it - 👍 |
When a share is confirmed the suggestions are got to check if there is an exact match. Usually the suggestions were already got with the same parameters in order to fill the autocomplete dropdown, so to avoid a superfluous request now the last suggestions are reused when got again, although only if the same parameters as the last time are used. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The suggestions depend on the results returned by the server, but also on the sharees already shared with. Due to that adding a share changes the suggestions, so now the cached suggestions are discarded when a share is added. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Before, whenever a pending operation (getting the suggestions, confirming a share or selecting a recipient) finished the working icon was hidden and the confirm button was shown again, even if there were other pending operations (the most common case is typing slowly on the input field, as several operations to get the suggestions could pile if the server response is not received fast enough). Now, the working icon is not hidden until the last pending operation finishes. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
7b75530
to
203bf51
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.
Tested and works (even in IE11
Another one in our quest to have a Confirm function for text fields #1068
Needs some actual JS to work @nickvergessen @oparoz @MorrisJobke