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

Implemented flip view animation on CreditCardView #11

Merged
merged 7 commits into from
Jul 9, 2015

Conversation

GregSaintJean
Copy link
Contributor

  • Updated .gitignore
    *Changed package name in sample to correct package name in build.gradle
    *Implemented flip animation for CreditCardView. Library will now require NineOldAndroids

Conflicts:
	library/build.gradle
	library/library.iml
	library/src/main/java/com/vinaygaba/creditcardview/CreditCardView.java
	library/src/main/res/values/attrs.xml
	sample/sample.iml
	sample/src/main/res/layout/activity_main.xml
@vinaygaba
Copy link
Owner

@GregSaintJean This is awesome! Will take some time to test it out tomorrow morning(1 AM here) and will merge it then :) Good job!

@GregSaintJean
Copy link
Contributor Author

Thank you. Let me know if you need any changes. I'll be more than happy to oblige.

@vinaygaba
Copy link
Owner

Hi @GregSaintJean ,
This is great! I really see a lot of value in this! What I would also like to see for a user to treat the back view as a layout rather than a static image. Is it possible for us to add fields, for example: CVV. If we are able to add more fields, it would really make it much more useful. What are your thoughts about this? I love the background image you used and that could be the default image provided by the library, with minor edits ofcourse(the numbers need to be removed if we let the user enter it).

@GregSaintJean
Copy link
Contributor Author

It’s very possible to programmatically give the user the ability to customize the front and back but you have to store both two images and fall back to defaults if the user didn’t prove their own background. I wouldn’t be too sure on how to handle the android:background attribute and the setBackgroundResource attribute since it’s original purpose was to help display one background at a time. I can only really think of overriding the method to accept the front background, with some help from the AOSP, and then creating a app:flipBackground attribute and setFlipBackground method to store and display the back of the card.

The default back image of the card would also have to be redone to support all the different screen densities. DPI values for each density would have to be done so that the views that overlay the back image will fall on the image perfectly.

For example, in the case of the CVV, currently the default image has numbers on it right now. It won’t take too much other than maybe some GIMP/Photoshop work to remove them. The numbers on the default back image right now provide as a placeholder in the meanwhile so we know where the CVV would be placed when we implement the view. So now we have to write preset DPI values to make sure that EditText view that is being used to input and display the CVV would be where the current placeholder is.

There would also be an additional problem of whether or not the custom image the user provides would also work with the preset DPI values. Just because the work that was done to make everything fall into place with the current default back image does not mean that the image the user provides will work either. The user would have to provide an image hopefully with the exact length and width as the default back image if the user is a developer.

If you want to give the user the ability to provide their own image through using the camera or pulling from the cloud/gallery, you could. You would just have to resize the image after you acquired it and then place it appropriately. If you did this however, you would be praying that the image is perfect enough after the resizing that everything falls into place.

You could provide the option of giving a custom image by putting an icon right next to where the flip button currently is right now. However, I would leave that as a separate issue from animating the card to flip. it would be work for another couple of days.

@vinaygaba
Copy link
Owner

Thank you @GregSaintJean for the elaborate explanation. The animating aspect that you have implemented works perfect across versions so that is a hige contribution as flipping views can be a bit tricky. So kudos to you for that.

Now coming to the issue of displaying additional fields, what I would be worried about is depending on DPI values. That would make things really static and definitely not the ideal way to do things as there will always be devices where it won't be displayed correctly. What we could do instead is create views that replicate the things we need at the back, much like what I did for the elements on the front. So for example, the magnetic strip could be a rectangle shaped drawable that wont be too difficult to implement. This, I think, could be a good solution to the problem.

And the backview could be a part of a separate layout file, which we change at the time of flipping to the backview (Not sure if this is a good/possible solution). Another approach could be all the elements from the front and the back view are on the same layout and their visibility is toggled based on which view is currently visible. Ofcourse, we would be changing the background resource like you are doing in the current implementation.

Let me know your thoughts. I trust layout elements more when compared to elements on an image any given day :)

@vinaygaba
Copy link
Owner

And I think we can let the users provide the option to select a custom back view. We can create a new attribute which would take a drawable resource and replace the current backgroun resource at the time of toggle. As for letting the users click an image, that is an open issue, one that we can take up later on as we can use external SDK's(Card.io) to implement that functionality.

@GregSaintJean
Copy link
Contributor Author

@vinaygaba, I actually really like the idea of replicating the magnetic stripe with a view so that we more accurately place things where we need them. Nice idea.

As far as using two separate views, I tried that already when implementing the flip view. You might have better luck than I did however because there might be something I didn't notice or didn't think about. Initially when I worked on the flip view, i used a Framelayout and tried to swap out two fragments. I ran into a problem with the merge tag however.

I also tried two separate views with merge as the root tag. I can't remember exactly what was the problem. There are two ways I can think of to swap out the layouts. You can use setContentView or remove the child from the parent. Neither of them worked but I wish i saved that work to see what the issue was. On a side note, now I should start saving failed progress in git so I can just roll back and take a look at it later. I do wish for two layouts however instead of one because it would at least make things easier to handle.

@vinaygaba
Copy link
Owner

Yup I agree. I'll try giving the two layouts idea a shot and see where that goes! I haven't worked much with views to be honest so I am not sure if I fancy my chances but I'll defintiely try 👍

As for the back view, it needs to have a magnetic strip(rectangular drawable), an EditText for the CVV and some white dummy area(rectangular drawable) to the left of it to actually make it look exactly like a real credit card. Since there are not a whole lot of things needed, I am tempted to implement these views on the same layout and toggle the visibility based on the view shown. But this could be the last resort.

@vinaygaba vinaygaba mentioned this pull request Jul 1, 2015
@GregSaintJean
Copy link
Contributor Author

Alright, I'll get right on implement these things.

@vinaygaba
Copy link
Owner

Hey sure you can get started. Its pretty easy but sharing the link anyway - http://stackoverflow.com/questions/10124919/can-i-draw-rectangle-in-xml :)

@biddster
Copy link

biddster commented Jul 1, 2015

This all sounds great to me. I'd also like to see a mode where both the front and back of the card is visible at the same time. The back of the card would appear to have slid underneath the front of the card (and is slightly smaller?). This is to save screen real estate and to also prompt the user for input. That way they see precisely what's required for inputting the card details at all times.

Perhaps the back of the card could also slide down animated from behind the front of the card?

@GregSaintJean
Copy link
Contributor Author

Let me know if there is anything wrong with this pull request. @biddster I think that request you made should probably go under another issue. I just want to put in the flip animation.

@vinaygaba
Copy link
Owner

Hi @GregSaintJean,
I was waiting to implement that drawable magnetic strip, etc that we discussed to merge the pull request. Otherwise it would cause confusion to ppl cloning the repo as the functionality is not entirely in place. Let me know your thoughts.

@GregSaintJean
Copy link
Contributor Author

I deleted the old background image that I was using for the back of the card and replaced it with three drawables. One for the background, one for the magnetic strip and one for the signature/CVV. After mentioning that I could just place a drawable for the magnetic strip, I found that the background image I placed before was very crude.

So I thought that since I put that background image in the first place, I should probably create a better replacement and include what I was hoping was the magnetic strip and the EditText for the CVV you mentioned earlier..

@vinaygaba
Copy link
Owner

@GregSaintJean If I understand correctly, this is still work in progress right?

@GregSaintJean
Copy link
Contributor Author

Please please please PLEASE PLEASE PLEASE tell me what's wrong. No, I finished it for the most part. Please tell me how I can improve upon the work I've done.

@vinaygaba
Copy link
Owner

@GregSaintJean I'm so sorry Greg, I didn't realize you had completed what we had discussed as well. Extremely sorry about that. Giving it a shot and merging it in the next 30 mins. Extremely sorry about missing this and the delay.

@vinaygaba
Copy link
Owner

Hi @GregSaintJean ,
This is really impressive work! Good job! Just one thing, I am not able to see the cvv field. I tried populating it via xml too but I don't see it getting populated. Not sure why as I checked the code and things look good.

@vinaygaba
Copy link
Owner

@GregSaintJean Okay found the issue. I'll merge and it and then fix it! Good job again

@vinaygaba vinaygaba merged commit 7b29c1b into vinaygaba:master Jul 9, 2015
@vinaygaba
Copy link
Owner

Hi @GregSaintJean ,
I have fixed a few bugs. One issue that I haven't fixed yet is the one where you set any other background apart from the default background (cardbackground_sky), flip the card to the backview and flip the card again to the front view. At this point you will see that the front background changes to cardbackground_sky. I know there is a static line which is causing this. What we need to do is to save the attribute the user sets using the getBackground() method and use that value when we flip it back. The problem is that getBackground() gives a Drawable instead of a Resource ID. Still figuring out a way to deal with this problem. Do you have any suggestions?

@GregSaintJean
Copy link
Contributor Author

Android SDK Search

If you use the chrome tool I linked, you can search for anything in the Android Documentation and look at it's source code.

The reason why I referenced it is because that's how I'm looking at how they implemented View. It would appear in the View implementation that they store an attribute called mBackgroundResource to carry the resource id of the background view. They never take the mBackgroundResource attribute and store it anywhere special.

So, it actually would not be a bad idea to create two variables for storing front view. One could be a constant for the default view and the other could hold the attribute used to display the custom background view. The only problem with this is in memory, there would be two instances where the custom background view would be stored (mBackgroundResource and whatever you named the newer variable).

I actually wanted to do something similar for the back side of the card but I didn't want to take liberties in implementing things in the library. I could have created a mBackSideBackgroundResource and mBackSideBackground variable, a constant for the default and four methods for setting and getting the back side background image but I wasn't sure if that seem cumbersome and bloaty and I was not sure if I should have created a getFrontSideBackground and getFrontSideBackgroundResource that would just return getBackground but not the resource somehow because mBackgroundResource is private in the View implementation. In order words, there was complications in my reasoning and I'm not the one in charge of the library.

EDIT: and now I just thought of using reflection for pulling private variables..... so I could have done that too but I've heard using reflection has the cost of the performance hit but never actually measured it for myself.

@vinaygaba
Copy link
Owner

@GregSaintJean So for the back side, the users will be able to customize the colors of the card, magnetic strip, etc which is important. I am still unsure if we should let developers replace it with a drawable resource? What do you think? Ideally they should be able to customize that too since we have given them the option of customizing the front view.

What I am not sure about is how much that bloats up the library and if there is a way to keep that in check. Let me know your thoughts.

@GregSaintJean
Copy link
Contributor Author

Those all sound like great ideas. Giving developers more control over customization is always good in my opinion. It really depends on how you want to do it.

@vinaygaba
Copy link
Owner

I would prefer keeping the back and front views identical, which would mean having another attribute like app:backBackground to take the back view background if the user wants to enter. The cardbackground_canvas.xml could be renamed to denote it is the back view equivalent of cardbackground_plain.xml.

@GregSaintJean
Copy link
Contributor Author

Keeping the names consistent like that to show the relationship between the front and view is very good and the additional attribute would compliment the setBackBackgroundResource method very well in my opinion.

@vinaygaba
Copy link
Owner

Also one more small thing. If the card is not flippable, we can hide the flipping button itself to avoid confusion. I didn't check this functionality but I'm guessing this was not already implemented?

@GregSaintJean
Copy link
Contributor Author

It is implemented and is flippable by default. I'm looking at the code now and I seem to notice that if the use set flippable to false, the button initially won't become invisible. I thought I programmed that in, but I redid what I wrote more than once and may have left that out. I'm sorry about that. I was willing to fix it.

@vinaygaba
Copy link
Owner

Hey no problem at all! You have done an amazing job already! Its a really small thing which is easy to miss! Dont worry about it. Once we have both the sides done(including the customizable background for backview) we can release it as part of the next release. Will also redo some of the screenshots and gif since we have some cool animations to show off 😆

@vinaygaba
Copy link
Owner

@GregSaintJean Wanted to give you a heads up that I have implemented what we had discussed above. Will do some more testing, get the screenshots ready and release it in the next couple of days :)

@GregSaintJean
Copy link
Contributor Author

@vinaygaba Thanks. Good to know.

@ericmuigai
Copy link

was the cvv implemented?

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.

None yet

4 participants