Skip to content
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

Configure how often upload progress event emitted, as well as UI revamp #18

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

joelwass
Copy link

The justification for the UI revamp is as follows:

  • Quite frankly I feel like only the transfers that occur in the current session use of the application should be logged - and not the complete history of the transferUtility.
  • I also logged in a more modular fashion so that users can look back on the progress, states, and file directories even after the upload/download succeeded or failed. Previously, once the upload failed or succeeded, the UI made it impossible to see the log of what occurred in the process, for example an error.
  • I additionally increased error logging visibility and the logging of both the s3 path of where the upload goes as well as the local file path of where a download goes.

@joelwass
Copy link
Author

ah hold up on this, only implemented in Android, will implement in iOS

@jhen0409
Copy link
Member

Thanks! I'll take time to review it. 😄

sendEvent("@_RNS3_Events", result);
getReactApplicationContext()
.getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class)
.emit("State_Changed", result);
Copy link
Member

@jhen0409 jhen0409 Jul 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just sendEvent("State_Changed", result)? There are different?

Also, I prefer to use @_RNS3_ as a event name prefix, we must ensure it don't conflict with other libraries.

@jhen0409
Copy link
Member

Currently we handle @_RNS3_Events and save progress to AsyncStorage, then call subscribers, let iOS version work well like Android version (because they are different 😂), it looks your PR will break this behavior.

I think we should keep the old way and split event into@_RNS3_STATE_CHANGED, @_RNS3_PROGESS_CHANGED, @_RNS3_ERROR. @joelwass, what do you think?

@joelwass
Copy link
Author

I think that's a great idea

On Jul 27, 2016, at 11:49 PM, Jhen-Jie Hong [email protected] wrote:

Currently we handle @_RNS3_Events and save progress to AsyncStorage, then call subscribers, let iOS version work well like Android version (because they are different 😂), it looks your PR will break this behavior.

I think we should keep the old way and split event into@_RNS3_STATE_CHANGED, @_RNS3_PROGESS_CHANGED, @_RNS3_ERROR. @joelwass, what do you think?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@joelwass
Copy link
Author

Any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants