Code-behind fragment and widget access fixes #1378

grendello · 2018-03-07T17:29:33Z

Fragment handling code partially taken from #1302

Fragments are treated as normal widget, with the difference that they have to be
found using FragmentManager.FindFragmentById instead of the usual
Activity.FindViewById method.

Each generated class also gets a property named Widget which returns the
actual Android widget as found in the layout file. This allows us to keep
hierarchical nature of the XML code (thus handling nested widgets with duplicate
ids gracefully) while being able to access the parent widget itself.

The commit introduces a new attribute called tools:managedType which is used
to specify the element associated property's type. We cannot use tools:class
for this purpose since the layout root element already uses it to specify the
name of the generated code-behind class and if the element had android:id on
it, we would end up using the activity's type for the root element's property in
the generated code, instead of its actual type.

jonpryor · 2018-03-07T17:49:58Z

Missing: Documentation update. ;-)

There's also a philosophical question which we've debated on (private?) Slack and Issue #1302: What API are we actually providing, and should this be consistent?

For example, if a .axml file has this fragment:

<TextView
    android:id="@+id/first_text_view"
    android:layout_width="wrap_content"
    android:layout_height="wrap_content"
    android:editable="false"
/>

What API are we exporting? There are three options, one of which must be documented:

Option 1: "Direct" binding- We bind as it is in the .axml file:

internal TextView first_text_view {get;}

This affords the "obvious" usage:

protected override void OnCreate (Bundle bundle) {
    first_text_view.Click += (o, e) => { /* ... */ };
}

However, this option won't -- can't! -- support .axml files which have multiple //@android:id elements with the same value. Presumably, we'd bind the first one found.

Option 2: "Indirect" binding: we bind as an indirect type in the .axml file:

internal __first_text_view_Views first_text_view {get;}
internal class __first_text_view_Views {
    public TextView Value {get;} // name?
}

This doesn't support the obvious usage of Option 1, but means that if the .axml declares multiple elements with the same //@android:id attribute, we can list them all in a consistent manner.

Usage is...weird:

protected override void OnCreate (Bundle bundle) {
    first_text_view.Value.Click += (o, e) => { /* ... */ };
    // WTF is `Value`? What do we call the "other" `//@android:id` "overloads"? Value2, Value3, etc.?
}

Option 3: Do both! Use Option 1 when there's only one //@android:id value within the file, and Option 2 otherwise.

This seems "obvious", but means there is no consistent API; it's dependent on how many //@android:id elements share the same value.

This could be acceptable -- maybe even preferable?! -- but will complicate the documentation process.

jonpryor · 2018-03-07T17:50:28Z

src/Xamarin.Android.Build.Tasks/Tasks/GenerateLayoutCodeBehind.cs

@@ -121,7 +122,7 @@ public override bool Execute ()

 		void GetLineInfo (IXmlLineInfo linfo, out int line, out int column)
 		{
-			if (linfo == null || linfo.HasLineInfo ()) {
+			if (linfo == null || !linfo.HasLineInfo ()) {


I half wonder if this should be a separate PR.

Nah. It's a one-liner and really doesn't deserve special treatment.

grendello · 2018-03-07T19:20:25Z

For example, if a .axml file has this fragment:
<TextView
   android:id="@+id/first_text_view"
   android:layout_width="wrap_content"
   android:layout_height="wrap_content"
   android:editable="false"
/>
What API are we exporting? There are three options, one of which must be documented:

Option 1: "Direct" binding- We bind as it is in the .axml file:

internal TextView first_text_view {get;}

This affords the "obvious" usage:
protected override void OnCreate (Bundle bundle) {
  first_text_view.Click += (o, e) => { /* ... */ };
}
However, this option won't -- can't! -- support .axml files which have multiple //@android:id elements with the same value. Presumably, we'd bind the first one found.

This option also doesn't take into account the hierarchy. I realize we can flatten it, but then you lose the contextual information from the axml and I think having this hierarchical context is, in fact, an advantage of our implementation over Android.

Usage is...weird:

protected override void OnCreate (Bundle bundle) {
  first_text_view.Value.Click += (o, e) => { /* ... */ };
   // WTF is `Value`? What do we call the "other" `//@android:id` "overloads"? Value2, Value3, etc.?

I don't know about "weird" here. This is used in all kinds of OOP GUI libraries where there's the notion of a container and its children or properties. As for the "other" overloads - the problem doesn't exist here because the other ones are nested in other parents and there's always unique and uniform path to access them:

protected override void OnCreate (Bundle bundle) {
   // MyTextView is the .axml widget id
   first_layout.MyTextView.Widget.Click += (o, e) => { };
   second_layout.MyTextView.Widget.Click += (o, e) => { };
}

As for sibling widgets with the same id, they aren't directly accessible in Android either - the first one is picked and we bind that one. The other dupes are reachable by enumerating children of the layout/parent/container widget.
Note: I used Widget above as the commit implements because I think it better implements the "meaning" of the property. Also, Widget exists only if the addressed widget has child widgets, otherwise it's a leaf widget and it has the actual Android type. This is pretty consistent and easy to document.

}

Option 3: Do both! Use Option 1 when there's only one //@android:id value within the file, and Option 2 otherwise.

This seems "obvious", but means there is no consistent API; it's dependent on how many //@android:id elements share the same value.

This could be acceptable -- maybe even preferable?! -- but will complicate the documentation process.

You seem to focus only on the duplicate IDs, but they are just part of the entire picture. I really think that the biggest advantage of my approach is the reflection of the hierarchical nature of the layout and it blends in nicely within the overall OOP paradigm of XA.

jonpryor · 2018-03-08T14:50:16Z