Skip to content
This repository has been archived by the owner on Apr 9, 2021. It is now read-only.

Default transferring mechanism and UI improvements #290

Merged
merged 23 commits into from
Aug 18, 2019

Conversation

huangyz0918
Copy link
Contributor

@huangyz0918 huangyz0918 commented Aug 3, 2019

Closes #266

What has been done to verify that this works as intended?

I tested this PR in my Nexus 6P (android 9) and Samsung Galaxy Note 3 (android 5).

Why is this the best possible solution? Were any other approaches considered?

This PR is still work in progress, there are some tasks we need to do:

  • add a default option in preference page.
  • remove the dialog that asking for choosing methods.
  • add the notification to ask if user need to switch method.
  • improve the UI and make the bluetooth and the hotspot different.

Extra improvements:

  • add a common method for present the results in a dialog. (finish the bluetooth activity after transmission completed)
  • add some animation in the bluetooth sending and receiving pages, make a better UI.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Before submitting this PR, please make sure you have:

  • run ./gradlew checkCode and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.

@huangyz0918 huangyz0918 changed the title [WIP] Default transferring mechanism [WIP] Default transferring mechanism and UI improvements Aug 3, 2019
@huangyz0918
Copy link
Contributor Author

huangyz0918 commented Aug 6, 2019

Note, I have done crazy tests in the switching method option, and commit 3d69968 still has some issues, the biggest issue is that when I switched the method from bluetooth -> hotspot -> bluetooth -> hotspot -> bluetooth... and something like that, there will be an exception like socked closed, I think this related to the socket handling, and should be reviewed detailed here.

And I think it will be good to show an AlertDialog for user to ask them if they really want to switch method, so I built the menu option for both of the sender and receiver. And I think if user face some issue during the transferring progress, he can stop and click the option in menu to switch method.

@huangyz0918
Copy link
Contributor Author

huangyz0918 commented Aug 9, 2019

@lakshyagupta21 I changed the colors in BtSenderActivity.java and BtReceiverActivity.java to blue and leave other pages the same as the original, what do you think about these changes?

I have three proposals

  • A blue color for BtSenderActivity.java and BtReceiverActivity.java, green for others.
  • A blue color for BtSenderActivity.java and BtReceiverActivity.java, green for HpSenderActivity.java and HpReceiverActivity.java, and another color for other pages (MainActivity.java, etc).
  • Apply the different color to all pages according default methods in settings.

Which one is better? @lakshyagupta21 @shobhitagarwal1612

Blue color looks like:

After clicking the switch icon in the menu, user can see a dialog, and can switch transferring method quickly.

@lakshyagupta21
Copy link
Contributor

I don't think the theme will help that much, because only toolbar color looks changes, which doesn't catch the user's attention much.
To catch user's attention either the whole screen should be colored (which I'm not sure whether that will look good or not) or adding some icon (bluetooth or hotspot) with different color.

@huangyz0918
Copy link
Contributor Author

I don't think the theme will help that much, because only toolbar color looks changes, which doesn't catch the user's attention much.

I changed the colorPrimary and some other colors so the button color even the progress bar color will change. Yeah, if it's not enough, I will add the icons and let's discuss more about how to make that looks better.

@lakshyagupta21
Copy link
Contributor

I changed the colorPrimary and some other colors so the button color even the progress bar color

A progress bar and buttons would be shown only when the transmission is started or when its scanning for other devices, right? So I think the first step is to identify which transfer mechanism is chosen by sender how will you identify at receiver that what mechanism sender has chosen?

@huangyz0918
Copy link
Contributor Author

huangyz0918 commented Aug 10, 2019

how will you identify at receiver that what mechanism sender has chosen?

Our receiver cannot know which method the sender selected. But we can build a discriminative UI that can tell users the methods are different. So what's your opinion about building that? I have three proposals above and what do you think about them? @lakshyagupta21

@huangyz0918 huangyz0918 changed the title [WIP] Default transferring mechanism and UI improvements [MRG] Default transferring mechanism and UI improvements Aug 10, 2019
@huangyz0918
Copy link
Contributor Author

@opendatakit-bot label "needs review"

@getodk-bot
Copy link
Member

ERROR: Label "needs review" does not exist and was thus not added to this pull request.

@lakshyagupta21
Copy link
Contributor

@huangyz0918 Transfer is not working when bluetooth option is selected. It keeps shows Connecting state.

Our receiver cannot know which method the sender selected. But we can build a discriminative UI that can tell users the methods are different. So what's your opinion about building that? I have three proposals above and what do you think about them? @lakshyagupta21

We should not change any theme or activity color, that should be kept the same for all activity.
As much I remember in our call last time we all agreed to add some icons on the screen with different color, For BlueTooth on both the screens (receiver's and sender's) blue color bluetooth icons should be there and some other different color(dark) for a hotspot. Or maybe we can do one thing change the color of the whole screen (to blue for bluetooth and some other dark color for hotspot) and add the white color UI elements.

@huangyz0918
Copy link
Contributor Author

huangyz0918 commented Aug 12, 2019

add some icons on the screen with different color

I see, so where should we place the icons? @lakshyagupta21

In the tool bar? I'm afraid only icons in the tool bar is not so obvious, how about adding in dialog like this?

@lakshyagupta21
Copy link
Contributor

@huangyz0918 Dialog is shown at the second step, we're trying to solve problem when user is on the screen and doesn't know about the transferring medium chosen, and only changing the theme won't work.

Here are few mocks I've created to convey our idea in better way, so that you can understand the requirement better.

Screen1
Screen2

Colors and text can be changed at a later stage.

Maybe @shobhitagarwal1612 @lognaturel @yanokwa Can give more suggestions to your ideas.

@huangyz0918
Copy link
Contributor Author

Transfer is not working when bluetooth option is selected. It keeps shows Connecting state.

Can you offer some more information like how to reproduce and error logs? @lakshyagupta21
Thanks! I'll try to fix issues as soon as possible.

@huangyz0918
Copy link
Contributor Author

Here are few mocks I've created to convey our idea in better way, so that you can understand the requirement better.

So do we need to reactor the UI for hotspot pages? @lakshyagupta21 I think there are many components already in that pages (like there are already a QR code for hotspot sender, no much room for a big logo, and that will make bluetooth and hotspot sender inconsistent), it's hard for us to build a page for hotspot like you proposed. As for me, I think adding an icon in the title bar is enough for user to know which method he selected. What do you think?

@shobhitagarwal1612 @lognaturel @yanokwa

@lakshyagupta21
Copy link
Contributor

As for me, I think adding an icon in the title bar is enough for the user to know which method he selected.

I'm not sure adding an icon in the toolbar just to catch the user attention is a good android design practice, but let's see what others have to say about it. Maybe in future, we may come up with QR Code feature for Bluetooth as well so we've to consider that as well.

@huangyz0918
Copy link
Contributor Author

huangyz0918 commented Aug 14, 2019

Cool, let see what others said @lakshyagupta21 .
For receivers, both bluetooth and hotspot have a recycler view for many items (devices list for bluetooth and wifi list for hotspot, it's also an issue if we want to add a big icon in the page), I don't know how to make the methods more obvious for our users, maybe change color for the whole page? I'm not sure that is a pretty look.

@lakshyagupta21
Copy link
Contributor

For receivers, both bluetooth and hotspot have a recycler view for many items (devices list for bluetooth and wifi list for a hotspot, it's also an issue if we want to add a big icon in the page)

We won't be adding an icon to list page, that page is dedicated to select the device with whom you want to make the connection, the icons and colors will be changed of only sender's screen. There is nothing that the sender can do by knowing which medium receiver has chosen, this feature is for the receiver to know that which medium sender has chosen so that he can switch the transfer medium

@huangyz0918 huangyz0918 changed the title [MRG] Default transferring mechanism and UI improvements [WIP] Default transferring mechanism and UI improvements Aug 14, 2019
@huangyz0918 huangyz0918 force-pushed the issue#266-default-mechanism branch from b291c10 to a0a3445 Compare August 15, 2019 00:46
@huangyz0918 huangyz0918 changed the title [WIP] Default transferring mechanism and UI improvements Default transferring mechanism and UI improvements Aug 15, 2019
remove the targetActivity and replace with this; close the bluetooth and the wifi after switching to
another method; add credits to icons from the internet.
@lakshyagupta21
Copy link
Contributor

Enable Secure Mode doesn't seems to be working I've enabled that in sender's device but not in receiver's device but still it asked me to pair using code.
And @huangyz0918 now the UI is looking pretty amazing, thanks for making it good.

@huangyz0918
Copy link
Contributor Author

doesn't seems to be working I've enabled that in sender's device but not in receiver's device but still it asked me to pair using code.

Yeah, the Enable Secure Mode was controlled by receiver, change settings in sender cannot affect the behavior. @lakshyagupta21

@lakshyagupta21
Copy link
Contributor

Yeah, the Enable Secure Mode was controlled by receiver, change settings in sender cannot affect the behavior. @lakshyagupta21

Can you briefly mention that in the settings?

@huangyz0918
Copy link
Contributor Author

Can you briefly mention that in the settings?

Sure

@lakshyagupta21 lakshyagupta21 merged commit 20ec77c into getodk:master Aug 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Build the default transferring mechanism
3 participants