Skip to content
This repository has been archived by the owner on May 25, 2021. It is now read-only.

Display real type information in the State view instead of always "Object" #776

Merged
merged 3 commits into from
Nov 11, 2016

Conversation

clbond
Copy link
Contributor

@clbond clbond commented Nov 8, 2016

Now we retain the name of the type across backend->frontend serialization so that we can display the real object type in the "State" view. See:
screen shot 2016-11-08 at 12 25 30 pm

Resolves: #775

@mention-bot
Copy link

@clbond, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sumitarora and @xorgy to be potential reviewers.

@igor-ka
Copy link

igor-ka commented Nov 8, 2016

@clbond I am assuming we do not display primitives types like string or number because there is no constructor property?

@clbond
Copy link
Contributor Author

clbond commented Nov 8, 2016

@igor-ka We display primitive types using bt-property-editor, so their types would never be shown on the UI. We let the user edit primitive values.

@igor-ka
Copy link

igor-ka commented Nov 8, 2016

@clbond yes i understand, I am wondering if it makes sense to display both the (primitive) type as well as let users edit them. It seems like an overkill at first thought but i am wondering what you think about this?

Object metadata cannot be based on paths, has to be based on reference, because the same object can appear at multiple paths
  - Luckily our serializer retains object references across serializations, so this can work for us
@clbond
Copy link
Contributor Author

clbond commented Nov 8, 2016

@igor-ka I had to add a bit of code to this PR because the way I did metadata is a bit wrong. I was creating a map of Path -> Metadata, but of course an object can appear at multiple paths, in which case the metadata was wrong. So I removed the md5 package and all the path hashing code and replaced it with object references. Our serializer maintains object references across serialization (if the same object appears twice, it will use the same instance), so now the metadata system relies on object references instead of paths when determining what decorators to apply to an object. Sorry if this is confusing ... it's a confusing problem

cannot store component metadata keyed off values, because oftentimes
those values are scalars that are not unique keys that can be used in a
map. So now we store the component instance as the map key and then
retain a list of all properties and their metadata.
@clbond
Copy link
Contributor Author

clbond commented Nov 8, 2016

OK, I fixed the case I discussed above. Paths are no longer used as keys for the metadata structure. Additionally, we can't store object values as component metadata keys because oftentimes those values are scalar and not unique. So we use the component instance itself as the map key and then store a list of properties and their metadata and associate those with the component. This works across all use cases, across circular references, duplicate references, deep nesting of component references, etc. I am confident in this fix.

Copy link

@igor-ka igor-ka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clbond LGTM :shipit:

@clbond clbond merged commit f8a5cf4 into rangle:dev Nov 11, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants