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

Added dedicated GIF post view with controls #1126

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Neywiny
Copy link

@Neywiny Neywiny commented Sep 23, 2022

Premise:

I found myself wanting to pause GIFs or restart them. So, I looked into what options there were and saw that there was already a gif package that Infinity was using with a class that supported it. Never contributed to an open-source project like this, usually just making comments/issues. I really just want the feature implemented; I don't mind if the code is torn apart.

Implementation:

Originally was just going to replace the BigImageView, but in the interest of preserving full image functionality (such as zoom/pan), I split the views. This has the added benefit of simplifying things like hiding buttons depending on if it's a GIF or not, and a lot of the supporting code if not all of it already had hooks to differentiate the two. So, I just pointed the rest of the code to the correct class. Cleans up GIF views when images aren't viewed and vice versa.

Other things:

I also made some changes Android Studio recommended, things like bumping versions and whatnot in the Gradle and redoing some deprecated code.

Where I have suspicions of my own wrongdoing:

My Java is rusty, and I was designing this to be only for my phone which is still on Android 8. So, I'm sure there's a lot to do to make it better. I also am unfamiliar with app development past some light messing around, so a lot of the packages and addons and things are unfamiliar to me. Things like @Inject and @BindView. I tried my best, but I probably got some things wrong.

I may have messed up some things in the manifest, I'm not sure.

Thank you

app/build.gradle Outdated
@@ -3,7 +3,7 @@ plugins {
}

android {
compileSdk 32
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this?

Copy link
Author

Choose a reason for hiding this comment

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

That's likely because my phone is old and I was trying to get some things to line up with the debugger. I guess it should be changed back.

@@ -175,7 +176,9 @@ public interface AppComponent {

void inject(AccountSavedThingActivity accountSavedThingActivity);

void inject(ViewImageOrGifActivity viewGIFActivity);
void inject(ViewImageActivity viewGIFActivity);
Copy link
Owner

Choose a reason for hiding this comment

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

Should be viewImageActivity

void inject(ViewImageOrGifActivity viewGIFActivity);
void inject(ViewImageActivity viewGIFActivity);

void inject(ViewGifActivity viewGIFActivity);
Copy link
Owner

Choose a reason for hiding this comment

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

Should be viewGifActivity

app/build.gradle Outdated
@@ -110,7 +102,6 @@ dependencies {
def butterknifeVersion = "10.2.3"
implementation "com.jakewharton:butterknife:$butterknifeVersion"
annotationProcessor "com.jakewharton:butterknife-compiler:$butterknifeVersion"
compileOnly 'com.android.databinding:viewbinding:7.3.0'
Copy link
Owner

Choose a reason for hiding this comment

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

Cannot delete this

app/build.gradle Outdated
@@ -48,10 +44,6 @@ android {
enableSplit = false
}
}

buildFeatures {
viewBinding true
Copy link
Owner

Choose a reason for hiding this comment

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

Please enable this

app/build.gradle Outdated
@@ -29,10 +29,6 @@ android {
shrinkResources true
proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro'
}
debug {
Copy link
Owner

Choose a reason for hiding this comment

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

This should not be removed.

@Docile-Alligator
Copy link
Owner

Please merge the newest changes first.

@Neywiny
Copy link
Author

Neywiny commented Nov 6, 2022

Well I've been trying to find out how to do this for maybe 5 hours now. I really just want a few files changed. I don't know how to get git to just have the few files change on my dev branch. It keeps replacing files with old versions. I only need to change like a few files to get this done. I think if this one doesn't go through, I'll just give up on it.

@Wladefant
Copy link
Collaborator

Hey @Neywiny Sorry for the late reply. First of all never give up 😉. I think your solution would be to just use git merge and solve the merge conflicts by hand.

The first steps into the open source world are normally very difficult, but once you get the the heck out of it, the gate opens up. Would be glad to see you again in our project.

@adamantike
Copy link
Contributor

@Neywiny, if this change is still relevant, I can help you to rebase the branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants