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

Fix issue #1163: Crash on upload picture with right to left languages #1172

Merged
merged 27 commits into from
Feb 28, 2018

Conversation

diddypod
Copy link

Fix crash on trying to upload pictures while phone language is a RTL language

Fixes #1163

Error was caused because the positions of elements on the screen were hard-coded assuming LTR layouts. I fixed it by checking the layout direction and proceeding accordingly.

@diddypod diddypod changed the title Fix crash on upload picture with right to left languages Fix issue #1163: Crash on upload picture with right to left languages Feb 20, 2018
@neslihanturan
Copy link
Collaborator

Can you please resolve the conflicts, so that I can test it easier?

@nicolas-raoul
Copy link
Member

@diddypod Can you run git rebase origin/master as Neslihan suggested? Thanks! :-)

Bluesir9 and others added 23 commits February 21, 2018 14:10
…, the GPS data is extracted and attached correctly.
… languages (2 folders in design/screenshots, 8 files each). All screenshots are ordered and follow the same pattern with the current English ones on the Google Play app page.
* Made the following changes:
->Added OpenCV library to the project
->Added functionality to detect if an image being uploaded is too dark
->Added functionality to detect if an image being uploaded is blurred

* Made corrections and changes based on gradle checkstyle requirements

* Updated gitignore to remove binary files related to OpenCV from project

* Image blurriness detection was undone. Images are checked only for being too dark now

* Removed OpenCV documentation folder containing a lot of html files

* Removed unnecessary buildScript usage in build.gradle file for opencv library and also added abi splits

* Removed OpenCV library usages and references from project

* Removed OpenCV library folder from project
I haven't found a good page yet about taking screencasts
…, the GPS data is extracted and attached correctly.
@commons-app commons-app deleted a comment Feb 21, 2018
@codecov-io
Copy link

codecov-io commented Feb 21, 2018

Codecov Report

Merging #1172 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1172      +/-   ##
=========================================
- Coverage    3.81%   3.81%   -0.01%     
=========================================
  Files         125     125              
  Lines        5812    5821       +9     
  Branches      568     570       +2     
=========================================
  Hits          222     222              
- Misses       5575    5584       +9     
  Partials       15      15
Impacted Files Coverage Δ
...e/nrw/commons/category/CategorizationFragment.java 0% <ø> (ø) ⬆️
.../free/nrw/commons/upload/SingleUploadFragment.java 0% <0%> (ø) ⬆️
...fr/free/nrw/commons/media/MediaDetailFragment.java 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f869520...67d1df2. Read the comment docs.

@diddypod
Copy link
Author

@neslihanturan @nicolas-raoul I've rebased as asked. Please check.

@diddypod diddypod mentioned this pull request Feb 21, 2018
@neslihanturan
Copy link
Collaborator

@diddypod I want to merge this as soon as possible to merge #1162 as soon as possible. However this PR cas conflicts besides we saw several commits from other people means you need to rebase it. If you follow our document https://github.com/commons-app/apps-android-commons/wiki/Setting-up-dev-enviroment you will be able to create a mergable PR with no problem.

@diddypod
Copy link
Author

@neslihanturan, I rebased as asked. Sorry for the delay

@commons-app commons-app deleted a comment Feb 27, 2018
if (ViewCompat.getLayoutDirection(getView()) == ViewCompat.LAYOUT_DIRECTION_LTR) {
value = titleEdit.getRight() - titleEdit.getCompoundDrawables()[2].getBounds().width();
if (motionEvent.getAction() == ACTION_UP && motionEvent.getRawX() >= value) {
new AlertDialog.Builder(getContext())
Copy link
Member

Choose a reason for hiding this comment

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

the logic for creating a alert dialog is duplicated. It can be extracted out to a function.

Copy link
Author

Choose a reason for hiding this comment

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

That was careless of me, I overlooked it. I'll correct it and push immediately.

@commons-app commons-app deleted a comment Feb 27, 2018
@neslihanturan
Copy link
Collaborator

Thanks @diddypod ! Congratz your first time contribution:)

@neslihanturan neslihanturan merged commit b1a4f7f into commons-app:master Feb 28, 2018
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.

Crash on upload picture with right to left languages