SLCORE-1901 SLCORE-1895 SLCORE-1896 Introduce in-app notification for asking feedback #1641
Conversation
Krosovok
force-pushed
the
feature/vt/SLCORE-1896-show-message-request
branch
from
2ced92c to
65df6f4
Compare
|
|
||
| /** | ||
| * Display a message to the user, usually in a small notification. | ||
| * This message has options that user can pick from. Once user clicked on option, it's String key is returned. |
There was a problem hiding this comment.
'that user can pick' -> that the user can pick, or that users can pick
it's -> its
| /** | ||
| * Display a message to the user, usually in a small notification. | ||
| * This message has options that user can pick from. Once user clicked on option, it's String key is returned. | ||
| * If the user explicitly dismisses/closes the notification without clicking option, then it returns null. |
There was a problem hiding this comment.
what returns null? Is it they key that should be null? Or the entire response? If it's the latter, I'm not sure we did that in the past, so far we always provided a response object, and manage different cases in the response object (e.g. some null fields). But it's probably fine to change this approach
There was a problem hiding this comment.
Sophio mentioned in the refinement doc that for VSCode
If the user explicitly closes the notification, it will resolve with
undefinedvalue.
While I am not 100% sure if that means the whole response or just value, I am just coming from assuming the worst. Some Optional chaining will handle it easily anyway.
| * Display a message to the user, usually in a small notification. | ||
| * This message has options that user can pick from. Once user clicked on option, it's String key is returned. | ||
| * If the user explicitly dismisses/closes the notification without clicking option, then it returns null. | ||
| * IMPORTANT: As user might not react to the notification at all, the returned future might block for an indefinite amount of time. |
There was a problem hiding this comment.
as user -> as users
| * This message has options that user can pick from. Once user clicked on option, it's String key is returned. | ||
| * If the user explicitly dismisses/closes the notification without clicking option, then it returns null. | ||
| * IMPORTANT: As user might not react to the notification at all, the returned future might block for an indefinite amount of time. | ||
| * So caller should not block waiting for the result, but provide a callback instead. |
There was a problem hiding this comment.
caller -> the caller
I'm not sure I understand the advice, but it's fine, future callers will check the implementation to come
There was a problem hiding this comment.
Basically it means that it shouldn't be called from any thread that is supposed to handle anything else (like SLCore initialization for instance). As we are going to schedule it anyway it doesn't matter this time but I just wanted to make sure that people will not freeze SLCore by waiting for a user action.
Krosovok
force-pushed
the
feature/vt/SLCORE-1896-show-message-request
branch
from
65df6f4 to
2ad1f7c
Compare
Krosovok
changed the title
|
2 participants
No description provided.