Add a gesture function to the scrollview to insert the zoom-in out function. #873
Add a gesture function to the scrollview to insert the zoom-in out function. #873saechimdaeki wants to merge 11 commits intowallabag:masterfrom
Conversation
Add a gesture function to the scroll view to insert the zoom-in out function.
|
This pull_request is an attempt to make this app more convenient. #854 |
|
What's |
|
@tcitworld https://code.google.com/archive/p/android-zoom-view/downloads |
|
Unnecessary libraries have been removed. |
|
It's not in the |
|
@di72nn Sorry, I forgot to find it in Branch when I did it on another computer. It has now been deleted. |
di72nn
left a comment
There was a problem hiding this comment.
I'm sorry, but code-wise it's a mess.
I can give some pointers on how to clean this up, but I don't think we need such a feature in its current form (scaling a ScrollView).
I'd like to hear somebody else's opinion on whether we can use these "scale gestures" in ReadArticleActivity for something.
|
I usually watch game community sites often.But the files on the site sometimes look too small, so I added them because I needed an extension. I'm sorry for the mess. |
|
So you only need to zoom in on some particular details, not for a whole article? |
|
Then shall I add it as you say? If that's better and you don't mind. |
|
@tcitworld @Strubbl @NWuensche do you think we need such a feature? It seems the implementation can be pretty compact, so I have no strong feelings one way or the other. |
|
If you guys say it's okay, I'll try.If not, I won't. Tell me without reserve. |
|
I like the idea of the feature since i have some articles, which have graphics, where i cannot zoom in from wallabag. So i go to the original website and look for the pic.
This sounds like a solution to the described problem. |
| settings = App.getInstance().getSettings(); | ||
|
|
||
| if(settings.isFullscreenArticleView()) { | ||
| if (settings.isFullscreenArticleView()) { |
There was a problem hiding this comment.
it would be nice to have a pull request without all the whitespace changes. Otherwise it is hard to review.
Of course, we can have whitespace changes, but please in a different PR.
There was a problem hiding this comment.
sorry .. It looks like it came from using the reformat function of the Android studio.
| @@ -1 +0,0 @@ | |||
| ../values/strings.xml No newline at end of file | |||
There was a problem hiding this comment.
i do not get why this file was changed. can you please explain
There was a problem hiding this comment.
I deleted it arbitrarily because it was a string.xml file with nothing. Sorry that made it impossible to run on my computer.
There was a problem hiding this comment.
This is a symlink. values-en/strings.xml points to ../values/strings.xml because english translation is our default language.
There was a problem hiding this comment.
I put it back. but it make all checks have failed
There was a problem hiding this comment.
Here the TRAVIS CI shows all checks have failed. Can I get a review without this file for a while?
This reverts commit 91d4e68.
|
@saechimdaeki thank you for your work and your continued effort to fix it, but I think that if we decide to have this feature it would be better if I make a new PR based on yours with some changes I'd like to make. I would of course credit you for the initial idea and implementation.
Note that this way of implementing it seems to be doing simple scaling - it scales already rendered image - so no extra pixels. Particularly for images it may be better to try to implement "tap to open" (I think there's an issue for that). |
In my use case this would be okay because the image embedded to the article was scaled down. So in theory the pixels are already there if i could zoom in. I do not know if the simple scaling would only zoom into the smaller rendered image or if it zooms into the original image. |
The smaller rendered one. |
|
In that case the feature is useless. |
|
Speaking of words, I tried. |
Add a gesture function to the scroll view to insert the zoom-in out function.