Private network support #49

slipo · 2018-01-15T20:40:14Z

Adds configuration panes to add/delete private networks.

Can be used by developers running NeonDB + a private NEO blockchain.

This PR also adds the CoZ Testnet as a third default option.

To test, if you don't have a private network set up you can add a copy of testnet with URL:
http://testnet-api.wallet.cityofzion.io

Here's what the add page looks like:

Some things to test:

Adding network
Deleting network
Getting balance and other per-network functionality of the extension (except test invoke which I think needs work outside the scope of this PR)
The Send Invoke popup window.

…lopment

austinhinderer · 2018-01-15T20:43:07Z

I noticed that there is a generated sass cache file being committed here. Should we be avoiding committing generated files other than the package lock files?

…lopment

austinhinderer · 2018-01-15T20:46:30Z

__tests__/components/NetworkSwitcher.test.js

@@ -6,7 +6,12 @@ import NetworkSwitcher from '../../src/app/components/NetworkSwitcher'

 const setup = () => {
  const props = {
-    network: { name: 'MainNet' },
+    networkId: 0,


Can you explain the difference between the networkId and the networks map below it?

network/networkId is the currently selected network. Networks is a listing of all available networks.

Right but I'm wondering how they map to each other, or if they do at all.

state.network.id is the id for accessing the config object states.config.networks. you think we should just add it to state.config.currentNetworkId or something like that?

austinhinderer · 2018-01-15T20:50:50Z

src/app/components/AddCustomNetwork/index.js

@@ -0,0 +1,2 @@
+import AddCustomNetwork from './AddCustomNetwork'
+export default AddCustomNetwork


This could be condensed to export { default as AddCustomNetwork } from './AddCustomNetwork'

austinhinderer · 2018-01-15T20:54:39Z

src/app/components/CustomNetworkList/CustomNetworkList.js

+      }
+    })
+
+    let content = (<div>You have no custom networks defined</div>)


This could be condensed a bit.

const content = networkRows.length ? <div>{ networkRows }</div> : <div>You have no custom networks defined</div>

or even

const content = networkRows.length ? { networkRows } : 'You have no custom networks defined'

austinhinderer · 2018-01-15T20:55:37Z

src/app/components/CustomNetworkList/CustomNetworkList.js

+
+    const networkRows = []
+
+    networks.forEach((network, index) => {


It might be better to keep this kind of logic outside of the render function.

You're saying just throw it in a different method of the same class?

Yes, render functions are ideally just template expressions. All of the generation of children are best abstracted out to their own methods for easy testing and clear understanding of their functionality.

austinhinderer · 2018-01-15T20:55:58Z

src/app/components/CustomNetworkList/index.js

@@ -0,0 +1,2 @@
+import CustomNetworkList from './CustomNetworkList'
+export default CustomNetworkList


Same as above.

austinhinderer · 2018-01-15T20:57:52Z

src/app/containers/Config/Config.js

+      <div>
+        <TabBar
+          style={ {
+            position: 'absolute',


Can this be moved to a stylesheet?

austinhinderer · 2018-01-15T20:58:09Z

src/app/containers/Config/index.js

-      </div>
-    )
-  }
+import { connect } from 'react-redux'


Nice job here

austinhinderer · 2018-01-15T20:59:25Z

src/app/containers/Transactions/index.js

@@ -61,7 +62,14 @@ export default class Transactions extends Component {
    )
    return (
      // <ul style="overflow: hidden;">{listItems}</ul>


Can this be removed?

austinhinderer · 2018-01-15T20:59:41Z

src/app/containers/Transactions/index.js

-      <textarea readOnly style='border: 0; bottom: 0;' rows='20' cols='40' name='transactionList'>{listItems}</textarea>
+      <textarea
+        readOnly
+        style={ { border: 0, bottom: 0 } }


Should this be handled in some sort of reset or moved to CSS?

Not sure. I was just fixing this part so it worked. I think transactions needs a good once over in general.

austinhinderer · 2018-01-15T21:00:54Z

.eslintignore

@@ -1,2 +1,3 @@
 **/*.html
 build/*
+coverage/*


Is this something that is generated by the package or your local environment?

if you run tests with coverage, yes

slipo · 2018-01-17T13:27:25Z

For the sass file, I agree it shouldn't be in the repo. Removing causes builds to fail. I think it should be handled separately.

…lopment

austinhinderer · 2018-01-18T16:26:25Z

src/app/components/CustomNetworkList/CustomNetworkList.js

+  delete = (index) => {
+    const { deleteCustomNetwork, setNetwork, selectedNetworkId } = this.props
+
+    // If this is the current network they're using, reset to MainNet (index 0)


Is this comment still valid?

austinhinderer · 2018-01-18T16:36:58Z

src/app/containers/Config/Config.js

+import PropTypes from 'prop-types'
+
+import { TabBar, Tab } from 'rmwc/Tabs'
+import '@material/tabs/dist/mdc.tabs.min.css'


I'm in progress on cleaning up some code, and I haven't needed to include the component CSS along with it. Have you tried without?

austinhinderer · 2018-01-18T16:38:19Z

src/app/containers/Config/Config.js

+          activeTabIndex={ this.state.activeTabIndex || 0 }
+          onChange={ evt => this.setState({ 'activeTabIndex': evt.target.value }) }
+        >
+          <Tab style={ { display: 'table-cell' } }>Networks</Tab>


Lets use css classes for any styling we need to do. In addition table-cell isn't the best for layout, have you considered a flexbox alternative?

austinhinderer · 2018-01-18T16:38:54Z

src/app/containers/Transactions/index.js

-      <textarea readOnly style='border: 0; bottom: 0;' rows='20' cols='40' name='transactionList'>{listItems}</textarea>
+      <textarea
+        readOnly
+        style={ { border: 0, bottom: 0 } }


Can we put this in our stylesheet or a reset?

austinhinderer · 2018-01-18T16:39:04Z

src/app/reducers/config.js

+  },
+  [ActionTypes.ADD_CUSTOM_NETWORK] (state, action) {
+    const networks = { ...state.networks }
+    networks[uuidv4()] = { name: action.name, url: action.url, canDelete: true }


…when we also support neoscan

slipo added 3 commits January 15, 2018 07:51

Merge remote-tracking branch 'upstream' into development

afeba97

Merge branch 'development' of github.com:CityOfZion/NeoLink into deve…

3352e05

…lopment

Private network support

1a0d188

slipo added 2 commits January 15, 2018 16:02

Merge branch 'development' of github.com:CityOfZion/NeoLink into deve…

dbd0f0e

…lopment

Merge branch 'development' into privnet

152e94e

austinhinderer suggested changes Jan 15, 2018

View reviewed changes

slipo added 4 commits January 17, 2018 08:42

Review comments

6dce2cf

Merge branch 'development' of github.com:CityOfZion/NeoLink into deve…

af0e2b7

…lopment

Merge branch 'development' into privnet

bce0574

Improved redux object structure for network config

c179914

austinhinderer reviewed Jan 18, 2018

View reviewed changes

slipo added 2 commits January 18, 2018 20:36

Review changes + add apiType to networks redux object for the future …

2b7cf88

…when we also support neoscan

Ignore yarn.lock so it never gets committed

fa91f9b

slipo mentioned this pull request Jan 19, 2018

Properly validate send form #55

Merged

slipo merged commit 3171aaf into CityOfZion:development Jan 21, 2018

slipo deleted the privnet branch January 21, 2018 18:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Private network support #49

Private network support #49

slipo commented Jan 15, 2018

austinhinderer commented Jan 15, 2018

austinhinderer Jan 15, 2018

slipo Jan 15, 2018

austinhinderer Jan 15, 2018

slipo Jan 15, 2018

austinhinderer Jan 15, 2018

slipo Jan 15, 2018

austinhinderer Jan 15, 2018

austinhinderer Jan 15, 2018

slipo Jan 15, 2018

austinhinderer Jan 15, 2018

austinhinderer Jan 15, 2018

austinhinderer Jan 15, 2018

austinhinderer Jan 15, 2018

austinhinderer Jan 15, 2018

austinhinderer Jan 15, 2018

slipo Jan 17, 2018

austinhinderer Jan 15, 2018

slipo Jan 15, 2018

slipo commented Jan 17, 2018

austinhinderer Jan 18, 2018

austinhinderer Jan 18, 2018

austinhinderer Jan 18, 2018

austinhinderer Jan 18, 2018

austinhinderer Jan 18, 2018

		@@ -0,0 +1,2 @@
		import AddCustomNetwork from './AddCustomNetwork'
		export default AddCustomNetwork


		const networkRows = []

		networks.forEach((network, index) => {

		@@ -0,0 +1,2 @@
		import CustomNetworkList from './CustomNetworkList'
		export default CustomNetworkList

Private network support #49

Private network support #49

Conversation

slipo commented Jan 15, 2018

austinhinderer commented Jan 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slipo commented Jan 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment