Skip to content

Display firebase data in data browser. make properties view look like the mocks.#9281

Merged
davidsbailey merged 9 commits into
stagingfrom
display-firebase-data
Jul 5, 2016
Merged

Display firebase data in data browser. make properties view look like the mocks.#9281
davidsbailey merged 9 commits into
stagingfrom
display-firebase-data

Conversation

@davidsbailey

@davidsbailey davidsbailey commented Jun 30, 2016

Copy link
Copy Markdown
Member

There is some semi complicated stuff going on here which maybe I should add in the comments regarding how and when we listen for data to update the various Data views:

  1. when we switch into properties view, we stop listening to everything else and then start listening for properties changes
  2. when we switch into table view, we stop listening to everything else and then start listening for changes to that table
  3. when we switch into overview view, we stop listening to everything
  4. when we switch into Data mode, we start listening as per 1-3 above

That said, I'm, going to have to change the way we listen to data soon to deal with a few issues, so maybe the comments don't need to be fleshed out until I finish doing that. TODO list:

  • call off() in a way that it only cancels the data browser's listener, preserving any listening done by a call to onRecordEvent in the running program
  • stop listening to data browser events when we leave Data mode (into Code or Design mode)

spec: https://docs.google.com/document/d/1UfBgHCisr_8jSt8zHFgJL-XrlLthTrXJQZJwoedGYg4/edit#heading=h.4ikfybcr5h8z

properties view:
screen shot 2016-06-30 at 3 14 23 pm

live update:
data-browser-live-update

@davidsbailey

Copy link
Copy Markdown
Member Author

@Bjvanminnen please take a look. no rush on this, as I most likely won't be responding to feedback before Tuesday.

Comment thread apps/src/applab/DataProperties.jsx Outdated
getKeyValueData() {
return Object.keys(this.props.keyValueData).map(key => {
return [key, this.props.keyValueData[key]];
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could make this even a little simpler as follows (up to you):

return Object.keys(this.props.keyValueData).map(key => [key, this.props.keyValueData[key]]);

or if you still want the extra line

return Object.keys(this.props.keyValueData).map(key => (
  [key, this.props.keyValueData[key]]
));

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This comment made me realize that getKeyValueData() is useless. I've taken what you suggested here and inlined it in render.

@Bjvanminnen

Copy link
Copy Markdown
Contributor

couple small comments, and looks like a legit test failure, but otherwise lgtm

@davidsbailey davidsbailey merged commit 1dd8eda into staging Jul 5, 2016
@davidsbailey davidsbailey deleted the display-firebase-data branch July 5, 2016 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants