-
Notifications
You must be signed in to change notification settings - Fork 38
feat(neon_framework): Support setting user status #1378
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
Conversation
packages/neon_framework/lib/src/widgets/user_status_dialog.dart
Outdated
Show resolved
Hide resolved
a960ad3 to
dd5074b
Compare
dd5074b to
32e94fb
Compare
|
Initial imoression: the picker looks beautiful 😍 |
32e94fb to
65e222b
Compare
|
Just setting the icon/emoji will not work here because of a bug in server: nextcloud/server#42532 |
65e222b to
559b8fd
Compare
|
(just a rebase) |
Leptopoda
left a comment
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.
559b8fd to
1511dcb
Compare
|
Putting the message input and clear at in the same line was a bit too complex and I think this now looks better than before. No tests yet, but apart from that it is done. |
|
I will rebase this onto #1418 once it is merged and change the clear at dropdown so it is more usable. |
2dfe44f to
1d8b042
Compare
1d8b042 to
70d099d
Compare
|
just a rebase, i still need to add the tests :/ haven't looked further into mocking the client or request manager. |
e135b7a to
c887956
Compare
|
@Leptopoda please review the bloc tests. I'm not sure yet if the way I did it is good, but I feel like it is the best we can do right now. Once we have more blocs with tests I'd like to merge the common logic, but there is no point in that right now. |
c887956 to
b6766cc
Compare
|
@Leptopoda added widget tests as well, this should now have all the tests we need |
b6766cc to
1072377
Compare
1072377 to
dc3571b
Compare
dc3571b to
e77164d
Compare
Leptopoda
left a comment
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.
If you want feel free to send the clearAt stuff as a separate PR as this looks good
e77164d to
2f2d585
Compare
Leptopoda
left a comment
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.
two things:
- please try to get a higher coverage in for the user status bloc
- return a 204 status code (should retry)
- return a not 204 status code (should emit error)
- the
if (!force && !result.hasError) {check correctStatuswith a predefined status set- whatever the line with
find(userId: username);
I mean you are able to read a coverage report :P
- On a fresh app start/hot restart the status remains in the loading state forever.
Signed-off-by: provokateurin <[email protected]>
Signed-off-by: provokateurin <[email protected]>
2f2d585 to
429abca
Compare
Signed-off-by: provokateurin <[email protected]>
Signed-off-by: provokateurin <[email protected]>
429abca to
61e57a8
Compare

Depends on #1375
Closes #442
TODO: