minderlabs / demo

Minder Demo App
1 stars 0 forks source link

Initial native commit #1

Closed TreyLawrence closed 8 years ago

TreyLawrence commented 8 years ago

Initial code to load json data from node server and display as list view


This change is Reviewable

richburdon commented 8 years ago
:lgtm:

Reviewed 40 of 41 files at r1, 1 of 1 files at r2. Review status: 40 of 41 files reviewed at latest revision, 17 unresolved discussions.


sub/NativeApp/common/containers/app.js, line 6 at r1 (raw file):

import { AppRegistry, TextInput, ListView, View , Text } from 'react-native';
import Search from '../models/search'

/* / comment (even if just class name).


sub/NativeApp/common/containers/app.js, line 8 at r1 (raw file):


export default class NativeApp extends Component {
  constructor(props) {

new line above methods.


sub/NativeApp/common/containers/app.js, line 1 at r2 (raw file):

'use strict';

make sure no tabs and 2 space indent. also TODO of format; TODO(tl): Full sentence.


sub/NativeApp/common/containers/app.js, line 2 at r2 (raw file):

'use strict';

rename containers => component? (prefer singular names). also app => list?


sub/NativeApp/common/containers/app.js, line 5 at r2 (raw file):

import React, { Component } from 'react';
import { AppRegistry, TextInput, ListView, View , Text } from 'react-native';
import Search from '../models/search'

newline separating their + our deps


sub/NativeApp/common/containers/app.js, line 10 at r2 (raw file):

  constructor(props) {
    super(props);
    const ds = new ListView.DataSource({rowHasChanged: (r1, r2) => r1 !== r2});

factor out rowHasChanged function (and base it off of item.id)


_sub/NativeApp/common/containers/app.js, line 10 at r2 (raw file):_

  constructor(props) {
    super(props);
    const ds = new ListView.DataSource({rowHasChanged: (r1, r2) => r1 !== r2});

Create abstraction for RemoteDataSource?


sub/NativeApp/common/containers/app.js, line 13 at r2 (raw file):

    this.state = { 
      text: 'Search',
      dataSource: ds

DS should be part of state (just update it below)


sub/NativeApp/common/containers/app.js, line 22 at r2 (raw file):


  results() {
    return fetch('http://10.0.3.2:8080/data/test.json')

document IP address in README (i.e., update our "guide")


_sub/NativeApp/common/containers/app.js, line 24 at r2 (raw file):_

    return fetch('http://10.0.3.2:8080/data/test.json')
      .then((response) => response.json())
      .then((responseJson) => {

rename json?


_sub/NativeApp/common/containers/app.js, line 25 at r2 (raw file):_

      .then((response) => response.json())
      .then((responseJson) => {
        return responseJson.items.map((result) => result.title);

rename result => item (can we bind item.id also to something?)


_sub/NativeApp/common/containers/app.js, line 29 at r2 (raw file):_

      .catch((error) => {
        console.error(error);
      });

TODO: Display error in UX


_sub/NativeApp/common/containers/app.js, line 36 at r2 (raw file):_

      <View style={{paddingTop: 22}}>
        <TextInput
          style={{height: 40, borderColor: 'gray', borderWidth: 1}}

Factor out styles or drop them


sub/NativeApp/common/containers/app.js, line 37 at r2 (raw file):

        <TextInput
          style={{height: 40, borderColor: 'gray', borderWidth: 1}}
          onChangeText={(text) => this.setSet({"text": text})}

spaces around {} (e.g., { foo: "text" })


sub/NativeApp/common/containers/app.js, line 37 at r2 (raw file):

        <TextInput
          style={{height: 40, borderColor: 'gray', borderWidth: 1}}
          onChangeText={(text) => this.setSet({"text": text})}

setState


sub/NativeApp/common/containers/app.js, line 40 at r2 (raw file):

          value={this.state.text}
        />
        <View style={{paddingTop: 22}}>

Remove inner View


_sub/NativeApp/common/models/search.js, line 3 at r2 (raw file):_

'use strict';

export class Search {

remove


Comments from Reviewable

richburdon commented 8 years ago

Reviewed 1 of 1 files at r3. Review status: all files reviewed at latest revision, 17 unresolved discussions.


Comments from Reviewable