Field ID generation #14
Conversation
|
Great! I appreciate the effort. I'll review the code soon and let you know of any feedback. |
|
|
||
| /** | ||
| * <code>FormModel</code> is an abstract class that represents the backing data for a form. It provides a mechanism | ||
| * for form elements to retrieve their values to display to the user and persist changes to the model upon changes. | ||
| */ | ||
| public abstract class FormModel { | ||
| public abstract class FormModel implements Serializable { |
There was a problem hiding this comment.
I wonder if there's another way we can maintain the model instance on rotations besides implementing the Serializable interface. It's reasonable, but my main concern is that it might be a bit too restricting for users if they need to implement their own model. This essentially means their model and its class members must be Serializable.
It might be a reasonable restriction of there's no other way, but I'd like to make sure we explore other options too. Were there other ideas you considered?
|
I mainly looked at the android documentation, which provides two ways of doing it:
I tried the second option and it works well, making the |
|
From those 2 options, I'd say approach 2 is definitely better. Would you please squash your commits to 1 so I can merge it? |
| @@ -11,7 +11,7 @@ | |||
| * {@link FormWithAppCompatActivity} | |||
| */ | |||
| public abstract class FormActivity extends Activity { | |||
|
|
|||
| private static final String MODEL_KW = "model"; | |||
There was a problem hiding this comment.
Not sure what the KW part stands for. What do you think of MODEL_BUNDLE_KEY as the constant name? Also MODEL_KEY works.
Also, let's prefix the value with nd_ (e.g. nd_model) to prevent collisions in case the the user also persists data in the Bundle.
There was a problem hiding this comment.
KW stood for keyword, I will change it to MODEL_BUNDLE_KEY.
Having static IDs made the application crash on rotation, because of the reconstruction of the FormActivity. This commit protect the form against this crash, generating unique ID for each view and saving the FormData (as a retained fragment) between Activity reconstruction.
aveuiller
force-pushed
the
fix-fieldIdGeneration
branch
from
ec0b4c4
to
3805358
Compare
fix crash caused by field view ID collision
|
Merged the changes to master. Thanks for your contribution @aveuiller! |
…models Feature/checkbox models
The fix wasn't as easy as expected, but I have something. :)
I had to create every field View programmatically, because xml layouts will end with the same bug as #12, i.e. every field of the same type will share the same id, so Android will recognize them as the same and copy values between them on rotation.
So the Idea is that every field has a new ID when created.
At this point, the rotation creates new fields (thus with different IDs), so their values weren't kept.
I had to make the
FormModelSerializable in order to save it upon Activity destruction and re-set it on activity creation (which are called on a phone rotation without special treatment).This will Close #11