-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Upload overhaul #1796
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
Upload overhaul #1796
Conversation
…. Cards show and hide, plus titles are correct. Displayed thumbnails for the shared images
… be shared with the new upload activity.
# Conflicts: # app/src/main/AndroidManifest.xml # app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java # app/src/main/java/fr/free/nrw/commons/di/ActivityBuilderModule.java # app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java # app/src/main/res/values/strings.xml
…isabling DUMMY UploadView object.
…ory screen now displays GPS and recent categories when nothing is searched.
…p#1849) * Add button on image details to copy wikicode to clipboard * Make copy wikicode button width the same as the nominate deletion button width by filling in background
…1859) * Eliminate the use of Picasso. This gets rid of the single use of the Picasso library (which was causing the whole library to be imported and shipped) and replaces it with Glide. TODO: replace this and the other instance(s) of Glide usage with Fresco, or vice versa. * Remove dependency on Glide. This removes the dependency on Glide, as well as the SVG rendering library, whose only purpose was to display a single SVG image in the Notification activity. Unfortunately Android doesn't support SVG natively, but Echo notifications have icons that are SVG formatted. Rather than import a bunch of heavy libraries to support this single case of SVG rendering, we can simply create a few local drawables that correspond to the different types of notifications, and use them instead. * Remove multidex! Multidex is a killer of performance and should be avoided at all costs. * Remove further unused bits. * Remove final vestige of multidex.
* Fix issue#1772 Add the CDATA tag to welcome_help_button_text string. Set the text of the corresponding textview using Html.fromHtml() function. * Fix issue#1772 Add the CDATA tag to nominated_see_more string. Set the text of the corresponding textview using Html.fromHtml() function.
app/src/main/res/values/strings.xml: Add the missing angle bracket.
* Added permission for Dexter, the runtime permission handling library * [Preparing fir issue commons-app#1773] Added a utility function which would take the user to app settings screen where he could manually give us the required permission * Added an alert dialog with positive and negative callback [Preparing fir issue commons-app#1773] * Improvements in the way External Storage Permission is handled in MultipleShareActivity[Bug fix commons-app#1697] 1. Used dexter to handle the external storage permission 2. Behaviour changes : When user tries to share(uppload) images to commons via MultipleShareActivity, following decision tree is followed a. If the app has permission for external storage, normal upload operation is followed b. If the app does not has the permission for external storage, dexter is used to ask for the same c. If the user gives us the required permission, normal upload flow is proceeded d. If the doesnot gives us the required permission a rationale dialog is shown with the appropriate message to let him know why we need the permission e. If he presses okay, steps a-c are followed and if he presses cancel, we close the app. f. If while asking for permission, the user chooses never ask again, then next time he tries to upload an image via MSA, the rational dialog follows the app setting screen where he could manually give us the required permission and the onActivityResult of same is handled * Added a Constants class to handle request and result codes from one place and other related constants common to the all app elements * replaced hardcoded strings ok and cancel in DialogUtil to string resources * init permission rationale dialog in activities onCreate * Code formatting, updated access modifiers wherever required, added javadocs for new methods created * *shifted constants to app class *Added JavaDocs in PermissionUtils * removed class REQUEST_CODES from CommonsApplication and instead put the enclosing constants in the App class itself
@ilgazer Am not sure if the PR is ready for review but just out of curiosity I was trying it out today and had trouble when I chose a photo from Google Photos. It kept showing "Receiving shared content, this may take a moment or two" and never proceeded. When I clicked a photo from the camera, it worked well. |
This should not be happening at all. I will look into it tonight. By the way, thanks for testing! |
@maskaravivek I failed to duplicate the bug, but this is after I made some changes to the received file caching mechanism. Could you please test again? Also, as the new UI has achieved feature parity with the old one at this point, should I finally delete the old upload activities and their dependent classes? I have hit the MultiDex method limit already and had to exclude the old Activities from the build as a workaround. Deleting the old activities at this point would allow more flexibility in modifying stuff like FileProcessor and moving some upload stuff to more logical places. |
Thanks for the update @ilgazer. I will try to test the PR tonight.
If the PR has achieved feature parity with master, should I test it extensively and list down any issues that I come across? |
I would love that! |
@ilgazer the PR works wells for me. I tried uploading photos using all the different flows ie.
In the process I also tested the following things:
There are a few issues that need some tweaking:
Also, feel free to remove the dead code and refactor pieces that should be taken up in this PR itself. |
It was supposed to be hidden. Fixed!
Added a toast.
Done!
Added!
The map view button shows the EXIF coordinates obtained from the file. The same feature was present under the expanding FAB in the Single Share activity. I will make it so that if there are no coordinates attached to the picture, the button will be hidden. Other than that, how could I make it more intuitive? |
I'm failing to duplicate the HEIC bug as the HEIC image isn't even recognized as an image. Could you tell me the steps you took between taking the photo on an iphone x and uploading them to commons? |
Added "Starting Upload" toast. Added link to the license on final screen. Made it so that the map button is hidden when image lacks gps coords.
3e69fd5
to
1421038
Compare
Thanks, @ilgazer. The above fixes work well for me. Just one more feedback. Right now, there is no check for an empty title while uploading. This check is there on the master. Can you add it? Also, it would be nice if you can include the latest changes from the master and make the PR up to date. This PR might stay open for a while before it is finally merged with master. :) |
Based on designs shared in #1128, this code introduces a new UploadActivity that aims to replace the old Share / Multi-Share activities. The basic categorization and upload functionalities are working but many features are still WIP. Also, anything and everything could be broken in the old ShareActivity but they will be gone soon anyway.