Improved ImageZoom screen #68
Conversation
|
@sagar15795 I haven't added the download and share image feature in this PR. Here is the demo of those feature : https://drive.google.com/file/d/1J-FPEhInuQLrP20mE_gAQwKjzfVBDHIc/view Let me know if it looks good to implement. I will implement those too. :) |
|
Hey, @saketkumar95 according to me the white background is perfect in image zoom. This black background looks odd to the application because the application theme is blue and white. So, I suggest you do not change the background colour inside image zoom. What do you think @sagar15795 |
|
@Hiteshgautam01 Yeah you are right but if you will check other applications like whatsapp or slack app. The ImageZoom screen is full black. The background as well as the status bar color. And It looks good to me according the google's new material design.
I think application theme doesn't matters here. I just tried to make it like a lightbox screen. https://material.io/guidelines/style/imagery.html#imagery-best-practices |
There was a problem hiding this comment.
@saketkumar95 Change the background and status bar using style.xml theme.
|
@sagar15795 Done! For download and share image option , I will create it in another PR. 👍 |
|
@saketkumar95 |
|
@sagar15795 Done! Please review now! :) |
|
@sagar15795 Here are the screenshots :
|
There was a problem hiding this comment.
@saketkumar95 Please change the requested changes. Also, follow the MVP architecture.
| @@ -225,4 +298,89 @@ public void onDestroyView() { | |||
| super.onDestroyView(); | |||
| mImageZoomPresenter.detachView(); | |||
| } | |||
|
|
|||
| public class DownloadImage extends AsyncTask<String, Integer, String> { | |||
There was a problem hiding this comment.
Use RxJava2 and Retrofit 2 to download a workflow Image.
asfgit
force-pushed
the
master
branch
2 times, most recently
from
03f68fd
to
f997f57
Compare
|
@saketkumar could you please remove this commit dea42ee or Use RxJava2 and Retrofit 2 to download a workflow Image so that we can merge it. |
|
yes sure @sagar15795 Just give me 1 day. I'm a bit busy with my work. I will complete it by thursday night. |
|
Do we really care if RxJava and Retrofit are used? If it works and is better then we can worry about that later. |
|
@ianwdunlop It will work. But it might crash the app sometimes because of the async task. |
|
Ok. Thanks for the advice @sagar15795 . How hard is it to change to RxJava/Retrofit? I assume there is a pattern in the existing code that uses it. |
This PR improves the ImageZoom Screen and added the animations of opening the activity.
Apply the
AndroidStyle.xmlstyle template to your code in Android Studio.Run the checks with
./gradlew checkto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.
Here is the screenshot after changes :
@sagar15795 Please review! :)