-
-
Notifications
You must be signed in to change notification settings - Fork 640
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 support for download and share. #660
Conversation
@Sam1301 Hey just one question, does this library supports both android and iOS files upload? As it will help for my work on uploading files. |
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.
Nice work @Sam1301 👍 :)
src/light-box/share.js
Outdated
export default (url: string) => { | ||
const shareOptions = { | ||
message: url, | ||
title: 'Shared using Zulip', |
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.
Can be added to src/i18n/translations/
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.
yep :), will add strings to transifex once pr is approved
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.
I think we need some better workflow for that since master
will technically be broken before translating this. (or not in that case?)
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.
👍
Here I need the translation as a string so probably I'd use injection api. Maybe we could do the translation outside this function? Did a quick test with this component.
src/api/downloadFile.js
Outdated
import { Auth } from '../types'; | ||
|
||
export default (url: string, auth: Auth) => { | ||
if (Platform.OS === 'android') { |
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.
can be done through this https://github.com/zulip/zulip-mobile/blob/master/src/styles/platform.android.js ?
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.
maybe not, since that setup is for controlling the stylistic differences on both platforms; example statusBar margin, etc.
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.
You can use any other file as platform specific though.
So downloadFile.ios.js
and downloadFile.android.js
oh, forgot to post the update message :) |
Heads up @Sam1301, we just merged some commits (latest: c1ca982) that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
Updated the branch. |
Good to go. |
Library used for download https://github.com/wkh237/react-native-fetch-blob.
It supports use of native download manager for android and file stream for large files in addition to features supported by https://github.com/itinance/react-native-fs.