Skip to content
This repository has been archived by the owner on Aug 16, 2019. It is now read-only.

Load Components from JSON #3

Closed
3 tasks done
MP0w opened this issue Dec 16, 2016 · 32 comments
Closed
3 tasks done

Load Components from JSON #3

MP0w opened this issue Dec 16, 2016 · 32 comments
Assignees
Projects

Comments

@MP0w
Copy link
Contributor

MP0w commented Dec 16, 2016

We want to be able to load the structure of an app from an external source.
We need to define a JSON schema that describes the Components.
The current idea is to define identifiers and the client will need to register Components in a factory, with this in place we will be able to refer to a Component builder from an identifier in the JSON.

Example:

{  
   "id":"tabbar",
   "children":[  
      {  
         "id":"myViewId",
         "meta":{  
            "foo":"bar"
         }
      }
   ],
   "meta":{  
      "selectedIndex":1
   }
}
  • JSON Schema
  • Component Factories
  • Serialization
@mathiasAichinger mathiasAichinger self-assigned this Jan 10, 2017
@mathiasAichinger
Copy link
Contributor

mathiasAichinger commented Jan 11, 2017

First Draft of the JSON. We introduced the field type, because it could be that a component is used multiple times with different data. So we will register factories for the types and ids should be unique in the JSON.

Open questions:

  • How to handle navigation controllers? Should we add an own component or use the meta field for it. Normally you have it for one branch once.
{
  "structure": {
    "id": "tabbar",
    "type": "tabbar",
    "children": [
      {
        "id": "history_tab",
        "type": "navigation",
        "children": [
          {
            "id": "stack",
            "type": "stack",
            "children": [
              {
                "id": "history.table_view",
                "type": "table_view",
                "meta": {
                  
                }
              }
            ]
          }
        ],
        "meta": {
          "tab_config": {
            "title": {
              "key": "history_title",
              "type": "localized_string"
            },
            "icon_name": "history_tab_icon"
          }
        }
      },
      {
        "id": "main_tab",
        "type": "navigation",
        "children": [
          {
            "id": "stack",
            "type": "stack",
            "children": [
              {
                "id": "test_label",
                "type": "label",
                "meta": {
                  
                }
              },
              {
                "id": "main.test_feature",
                "type": "test_feature",
                "meta": {
                  
                }
              },
              {
                "id": "main.start_button",
                "type": "button",
                "meta": {
                  "title": {
                    "key": "start_button_title",
                    "type": "localized_string"
                  }
                }
              }
            ],
            "meta": {
              
            }
          }
        ],
        "meta": {
          "tab_config": {
            "title": {
              "key": "main_tab_title",
              "type": "localized_string"
            },
            "iconName": "main_tab_icon"
          }
        }
      }
    ],
    "meta": {
      "selectedIndex": "1"
    }
  }
}

@MP0w
Copy link
Contributor Author

MP0w commented Jan 11, 2017

What's special about type that doesn't fit in meta?
For navigation controller: it looks like a wrapper (only one child) to me, in fact it is a ViewController that wraps another one!

@joanromano
Copy link
Contributor

Thoughts are that type will always be a mandatory field for every component, whereas meta is an optional field. @MP0w do you think it should still be within meta?

@MP0w
Copy link
Contributor Author

MP0w commented Jan 12, 2017

Probably I'm missing the point of type, examples?

@joanromano
Copy link
Contributor

Since ids should be unique, this would be a json draft:

{
    "structure": {
        "id": "tabbar",
        "type": "tabbar",
        "meta": {
            "default_tab_id": "main_tab"
        },
        "children": [
                     {
                     "id": "history_tab",
                     "type": "stack",
                     "meta": {
                     "tab_config": {
                     "title": {
                     "key": "history_title",
                     "type": "localized_string"
                     },
                     "icon_name": "history_tab_icon"
                     }
                     },
                     "children": [
                                  {
                                  "id": "history.table_view",
                                  "type": "table_view"
                                  }
                                  ]
                     },
                     {
                     "id": "main_tab",
                     "type": "stack",
                     "meta": {
                     "tab_config": {
                     "title": {
                     "key": "main_tab_title",
                     "type": "localized_string"
                     },
                     "iconName": "main_tab_icon"
                     }
                     },
                     "children": [
                                  {
                                  "id": "test_label",
                                  "type": "label"
                                  },
                                  {
                                  "id": "main.test_feature",
                                  "type": "stack",
                                  "meta": {
                                  "orientation": "horizontal"
                                  }
                                  },
                                  {
                                  "id": "main.start_button",
                                  "type": "button",
                                  "meta": {
                                  "title": {
                                  "key": "start_button_title",
                                  "type": "localized_string"
                                  }
                                  }
                                  }
                                  ]
                     }
                     ]
    }
}

Where types specify which component to load from the factory

@MP0w
Copy link
Contributor Author

MP0w commented Jan 12, 2017

Sorry I mean an example of a view that has same identifier but two different types.

"id": "history_tab",
"type": "stack"

Why is history_tab needed? Having a stack cluster is not enough? Or make history_tab a wrapper that contains a stack cluster?

@joanromano
Copy link
Contributor

We need unique identifiers per component, there was some reason but can't remember why know. @mathiasAichinger could you elaborate?

@MP0w
Copy link
Contributor Author

MP0w commented Jan 12, 2017

unique identifier is already id isn't it? that's why I don't get type.

For what I know ATM:

id: "tabbar"
children: ...
meta: ...
id: "myview"
meta: ...

are already identifying a unique view (Factories can build view from a given id)

@MP0w
Copy link
Contributor Author

MP0w commented Jan 12, 2017

What we need is just a way to retrieve information on how to build a Component from the json.
Given there are 3 types of Component ATM (view, wrapper, cluster)
We need Factories that can build them, retrieving a "builder" from a unique identifier (id)

We need something that given an idreturns a Componentbuilder:

// pseudocode
Factory["stack"] = {
    return Component(......)
}

Then if we need multiple stack the son can of course contain multiple stack:

"children": [
  {
     "id": "stack",
     "children": [
             {
                 "id": "stack",
                 .....
            }
     ]
      .....
  },
  {
     "id": "stack",
      .....
  }
]

So id doesn't have to unique in the json.
A stack is always a stack, no matter where it is or what it contains.

@joanromano
Copy link
Contributor

joanromano commented Jan 12, 2017

Yup, that's exactly the current approach we have with the type field . Let's discuss whether we need or not a unique identifier per component in the json then @mathiasAichinger. If we don't need it we can just remove it then.

@mathiasAichinger
Copy link
Contributor

The id and type idea came actually from the Android team, the understood the id differently. During the discussions I realized that it's maybe not bad to keep the id as unique id for the components. Currently there is no need for it, but maybe we need it later for eg. deep linking

@MP0w
Copy link
Contributor Author

MP0w commented Jan 13, 2017

I would say it should be added whenever there is a use case fit it but keep it out until it's useless

@markushi
Copy link

Actually we borrowed the meaning of id and type from the JSON API

The type member is used to describe resource objects that share common attributes and relationships.

The reason we suggest to use type instead of id is to avoid confusion.

@MP0w
Copy link
Contributor Author

MP0w commented Jan 13, 2017

Oh true, we could replace id with type then?

@markushi
Copy link

Yes I think for now we just need type, later we could introduce an (optional) id field if required.

@joanromano
Copy link
Contributor

Cool, so we agree on removing id and keep type then?

@MP0w
Copy link
Contributor Author

MP0w commented Jan 13, 2017

Yep. Let's keep it as simple as needed.
id can be introduced later if needed for anything.

@mathiasAichinger
Copy link
Contributor

Should we put the component configs into own json objects to have them kind of namespaced? Like

{
	"meta": {
		"tab_config": {
			"title": {
				"key": "history_title",
				"type": "localized_string"
			},
			"icon_name": "history_tab_icon"
		}
	}
}

@joanromano
Copy link
Contributor

@mathiasAichinger what would be the benefits? Definitely we would need more handling on the factories side so that they provide their meta name apart from their type name. But if it's worth the effort for the future we could consider that.

@mathiasAichinger
Copy link
Contributor

@joanromano Just to make it more clear for what this meta is used. But I agree to keep it simple at the beginning.

@MP0w
Copy link
Contributor Author

MP0w commented Jan 13, 2017

pretty sure it would make the materializations way harder

@MP0w
Copy link
Contributor Author

MP0w commented Jan 13, 2017

I think the specs are missing wrappers that have child instead of children

@joanromano
Copy link
Contributor

joanromano commented Jan 15, 2017

There has been recently a discussion brought by @MP0w in #7 regarding differentiation between children and child for cluster and wrapper respectively.

The question being, should we keep in the JSON schema children as an array of components no matter if it's a cluster or wrapper or should we differentiate and therefore have children for clusters as array of components and child for wrappers as single nested component?

@markushi @mathiasAichinger @AndreasThenn @MP0w let me know what are your thoughts. I believe, if I am not wrong, the idea behind having always children in the JSON was to match JSONAPI behaviour somehow.

@markushi
Copy link

In my opinion we should use the children array attribute for wrappers as well. It's a bit more verbose than just using child but keeps the parsing code simple and is less prone to possible errors.

E.g. in the case where you have some JSON where you want to switch from using a wrapper to a stack:
For the child approach

  • change type value to stack
  • change child to children
  • wrap existing child in JSON array

For children approach

  • change type value to stack

@MP0w
Copy link
Contributor Author

MP0w commented Jan 17, 2017

But we would break the type safety since it's an array instead of a single Component. That's not intuitive and more erro prone IMHO

@joanromano
Copy link
Contributor

@MP0w don't really see why is less intuitive and more error prone (maybe you could elaborate) since this would be defined by a JSON schema anyway.

I see also the benefits of having clear type definitions on the client side for clusters, wrappers and views, but not really on JSON itself since you should be aware of the schema anyway in order to parse it.

I would also go for having always children on the JSON for now since I believe it keeps things simpler.

@mathiasAichinger
Copy link
Contributor

@MP0w @markushi @joanromano
I would also go for the children approach, because it's simpler and I think it's even less error prone. You just use the first element in the array and so you can easy change a cluster to a wrapper.

@bassemawny
Copy link

Concerning the id vs type discussion:
In the root element (the tabbar), the meta is selected_index. Wouldn't it better be default_tab_id. And in this case we need the id attribute.
This would be a more flexible approach as the default tab would not be dependent on the ordering of the tabs and could then be used in some A/B testing where the order of the tabs is different but the default tab is still the same.

@bassemawny
Copy link

bassemawny commented Jan 17, 2017

Also we need to introduce another type of meta for the tabbar which is called title_state. It could be such that the icon titles in the tabbar are either always shown or shown only when active.
So something like that:

{
	"structure": {
		"id": "main_tabbar",
		"type": "tabbar",
		"meta": {
			"default_tab_id": "main_tab"
			"title_state": "show_when_active" // could also be "always_show"
		},
		"children": [{
			"id": "history_tab",
			"type": "navigation",
			"meta": {
				"title": "history_title",
				"icon_name": "history_tab_icon"
			},
			"children": [{
				"id": "history_stack",
				"type": "stack",
				"meta": {
					"orientation": "vertical"
				},
				"children": [{
					"id": "history.table_view",
					"type": "table_view"
				}]
			}]
		}, {
			"id": "main_tab",
			"type": "navigation",
			"meta": {
				"title": "main_tab_title",
				"icon_name": "main_tab_icon"
			},
			"children": [{
				"id": "main_tab_stack",
				"type": "stack",
				"meta": {
					"orientation": "vertical",
					"spacing": "5"
				},
				"children": [{
					"id": "test_label",
					"type": "label",
					"meta": {
					}
				}, {
					"id": "main.test_feature",
					"type": "test_feature",
					"meta": {
					}
				}
			]
			}]
		}]
	}
}

@MP0w
Copy link
Contributor Author

MP0w commented Jan 17, 2017

I don't really see why something that is not (and never will be) an array should be wrapped in an array.
Also I don't see how a wrapper can turn into a cluster since they are two different things and why we would like to make it "easier to change.
My point is: if I have to create a JSON describing a wrapper with children I'm going to think I can have many. But then those many children are just gone, never used (except the first).
"Child" doesn't add any complexity IMHO and makes things more explicit and clear.
The only thing way it could makes sense is to have clusters that only consider one child and remove wrapper, wrapper becomes a normal weird cluster (which a solution I don't like either)

@MP0w
Copy link
Contributor Author

MP0w commented Jan 17, 2017

@bassemawny using the id instead of index could be valid but then the cluster needs to understand what's the index. The argument of not depending by the ordering and AB test stuff is not valid IMHO because the selectedIndex is also coming with the JSON and you would have the same problem using identifiers (if the id of children changes you need to change the selectedIndex)

@joanromano
Copy link
Contributor

After discussion, for now we agreed on:

  • Keeping children for every component. We always have time in the future for a change to child in wrappers if it makes sense.
  • Keeping selected_index meta param for the tab bar's selected index.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Matrioska
In Progress
Development

No branches or pull requests

5 participants