-
Notifications
You must be signed in to change notification settings - Fork 321
Android keyboard image support #1643
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
base: main
Are you sure you want to change the base?
Android keyboard image support #1643
Conversation
Thanks for taking this on! I think pushing to forks requires maintainer access to the source repo. Closing #1203 in favor of this PR. |
7ed9277
to
60c73d1
Compare
60c73d1
to
4df9f93
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.
Thanks @sm-sayedi for picking this up, and @PIG208 for the original PR and @chrisbobbe for the previous reviews there!
Generally this looks great; just a few nits below.
lib/widgets/compose_box.dart
Outdated
filename: path.basename(content.uri), | ||
mimeType: content.mimeType); | ||
|
||
await _uploadFiles( |
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.
nit: see #1801 (once you rebase atop main)
lib/widgets/compose_box.dart
Outdated
void _handleContentInserted(KeyboardInsertedContent content, | ||
BuildContext context) async { |
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.
nit: when there's a context parameter, put it first
test/widgets/compose_box_test.dart
Outdated
attachedFileUrl: | ||
'content://com.zulip.android.zulipboard.provider' | ||
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif', |
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.
nit: indent
attachedFileUrl: | |
'content://com.zulip.android.zulipboard.provider' | |
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif', | |
attachedFileUrl: | |
'content://com.zulip.android.zulipboard.provider' | |
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif', |
(and a couple of others below)
test/widgets/compose_box_test.dart
Outdated
])), | ||
(ByteData? data) {}); |
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.
nit: indentation
])), | |
(ByteData? data) {}); | |
])), | |
(ByteData? data) {}); |
(otherwise it looks like another argument to encodeMethodCall; it's actually an argument to handlePlatformMessage)
Signed-off-by: Zixuan James Li <[email protected]>
So the test is more realistic. A side effect of this is that the content input text field is focused afterwards. Signed-off-by: Zixuan James Li <[email protected]>
Fixes: zulip#419 Fixes: zulip#1173 Signed-off-by: Zixuan James Li <[email protected]>
4df9f93
to
5561fca
Compare
Thanks @gnprice for the review. New changes pushed. |
#1203 but rebased on top of main.
Thanks @PIG208 for all your work in #1203. I am assigned to follow your work until merged. I tried to push to your branch, but didn't have the permission.
Keyboard image/gif
Keyboard.Image.GIF.mov
Keyboard paste
Keyboard.Paste.mov
Fixes: #419
Fixes: #1173