This repository has been archived by the owner on Jan 23, 2024. It is now read-only.
Fix and reformat drawables, v2 #490
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@Blaisorblade @Eremiell @sarav @nahojjjen test IV button please~ |
|
The button looks right on Nexus 5X and Nexus 7. |
| @@ -1177,7 +1177,7 @@ private void adjustSeekbarsThumbs() { | |||
| // Set Thumb 1 drawable to an orange marker and value at the max possible Pokemon level at the current | |||
| // trainer level | |||
| expandedLevelSeekbarBackground.getThumb(0).setThumb(getDrawableC(R.drawable | |||
| .orange_seekbar_thumb_marker)); | |||
| .orange_marker)); | |||
There was a problem hiding this comment.
This code is inside a Service, not an AppCompatActivity, so IIUC what @thearaks explained, it's going to fail (right)? Or does that issue not apply to getDrawable?
|
Review by @thearaks. But isn't this going to suffer from the usual issues on Android 4.4? Or are these drawables only used inside |
|
That's something I requested for testing by anyone with a KitKat device since v1 of the PR. I do have a work around if this implementation fails. |
|
Yeah you're right, but my only 4.4 device kind-of never works. |
|
Current workaround would be to rasterize the drawable into a bitmap, but only for KitKat. |
|
Tested on 4.4. We know that GoIV doesn't work well there, but this time it crashes as soon as I pressed Start (precisely, when starting Pokefly), which is much earlier. OTOH that's "good" since PoGo doesn't really run on this device (this Galaxy Nexus has ~700MB RAM).
That seems... tricky to do, unless you have yourself a device to test this with. Maybe this should just wait for the big refactoring? Meanwhile, how hard would it be to use these button sizes (and fix #402) without the rest? If that's too much work maybe this should wait. Stack trace from logcat follows: |
|
Sweet. That was exactly what I'm expecting to happen. Rasterizing to bitmap is just a 5 lines thing. No biggies. |
|
@Blaisorblade for your testing on 4.4! I found the actual reason but I also fixed a potential crash. |
| android:visibility="gone"> | ||
|
|
||
| </AutoCompleteTextView> | ||
|
|
||
| <Button | ||
| android:paddingLeft="4dp" | ||
| <android.support.v7.widget.AppCompatImageButton |
There was a problem hiding this comment.
Wait, that's what they want us to use? I didn't know XML allowed that...
|
No crash but more exceptions. Then I got a crash when stopping. Here's the crash on stop, it seems #477: |
|
Got it. I'll be back with more fixes! Thanks a lot for the testing. |
|
@DJCrapsody How much space is this saving? This looks like a lot of work. Hopefully, it's worth the time. |
|
Most svg can be generated through android studio itself so most of the time went to fixing the IV button |
|
Just noticed this PR drops the initialization of |
|
@Blaisorblade time for more 4.4 testing |
|
Regarding the button size, I've discovered something. You remember it worked on my phone? Today I changed the pixel size (or something like that)*, and GoIV's button shrank while PoGo didn't. I think they simply aren't using Android's pixel density, so maybe we should ignore it as well somehow.
|
|
Strange, doesn't sound right tho... Unless changing it changes the exact pixel counts. Could you take a screenshot for each level of non-default display size? |
I'm assuming it does shrink display pixels, and the text size control should shrink screen pixels. Here's a screenshot with master and the "Small" Display Size setting I mentioned:
Here it is with Normal size: Here it is with Large size: Here it is with Larger size: For extra fun, here's GoIV: Here it is with Largest size: |
Convert iv button drawable to layer-list xml with shapes Fix size and location (mostly)
Use app:srcCompat instead of android:src for vector drawables
Generalize method with getVectorDrawable() for vector drawables Readd initialization of pointerHeight and pointerWidth that was unintentionally removed
Magic numbers are calculated based on devices with standard PPI to get the estimated size/x/y of the IV button (not always accurate) getVectorDrawables is used when retrieving vector drawables
|
@Blaisorblade javadoc added and magic numbers are now constant |
|
Does this become more mergeable after #506? |
|
I noticed black screens (when pokego is already running) while testing this. I will check again when i get home. |
That sounds like the known black-screen issue — if PoGo is running, you start GoIV and switch to PoGo, sometimes PoGo goes black and you have to restart it. |
|
I'm closing this PR due to inactivity for cleanup. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR supersedes #433
PR consists of: