fidesmo / nordpol

The Android Support Library for NFC
MIT License
53 stars 10 forks source link

Introduce NfcGuideView #49

Closed Franzaine closed 8 years ago

Franzaine commented 8 years ago

This PR introduces NfcGuideView. NfcGuideView is an Android View that consists of a layout with a couple of child views. Depending on the state set with the public method: public void setCurrentStatus(NfcGuideViewStatus nfcGuideViewStatus){...} the view shows different states. The four states are defined in an enum:

public enum NfcGuideViewStatus {
        STARTING_POSITION,
        TRANSFERRING,
        DONE,
        FAIL
}
Franzaine commented 8 years ago

@slomo ready for review

slomo commented 8 years ago

Should we really add fidesmo specific assets to nordpol?

Franzaine commented 8 years ago

@slomo valid question, but I am currently adding this to OpenKeychain as per their request which means that others might be interested as well. That is at least the current thinking

Franzaine commented 8 years ago

@petter-fidesmo

ghost commented 8 years ago

@Franzaine Maybe these ones can be default but there could be an option to override?

Franzaine commented 8 years ago

@slomo ready for re-review! :)

Franzaine commented 8 years ago

@petter-fidesmo ready for review! I don't want to wait until Yves is working again😏

ghost commented 8 years ago

@Franzaine This is beyond my android knowledge. Is there a simple example on how this would be used? Maybe we should add it to the OTP tutorial?

ghost commented 8 years ago

@Franzaine Left some comments. Should we add this to nordpol-android or should we create a new project, nordpol-android-ui?

Franzaine commented 8 years ago

@petter-fidesmo I think we should keep it as is. It should not make a too big impact on the lib size and there is very little code (if we want to be afraid of the dex limit ;) ) Just like we could incorporate it we can extract it too, in the future. If we feel like it.

Franzaine commented 8 years ago

@petter-fidesmo if you agree with my above comment and like the change I did to usage (I explained it a bit more) I'd like to merge this.

ghost commented 8 years ago

@Franzaine :+1: