Web Based Dapps #3956

tomusdrw · 2016-12-22T17:52:37Z

Web Browsing dapp
Streaming content instead of using temporary files
Better timeouts handling for fetch
Protecting Web-based dapps endpoint with random token

What's left

Tests for fetching/streaming! (separate PR)

Testing: npm start and then http://127.0.0.1:3000/#/web

Conflicts: dapps/src/apps/fetcher.rs dapps/src/handlers/fetch.rs

Conflicts: Cargo.lock hash-fetch/Cargo.toml hash-fetch/src/client.rs rpc/Cargo.toml rpc/src/lib.rs rpc/src/v1/impls/parity_set.rs util/fetch/Cargo.toml util/https-fetch/Cargo.toml

Conflicts: dapps/src/apps/fetcher/installers.rs dapps/src/apps/fetcher/mod.rs dapps/src/apps/mod.rs dapps/src/handlers/fetch.rs dapps/src/lib.rs dapps/src/web.rs js/src/views/Web/index.js parity/dapps.rs parity/run.rs util/fetch/src/client.rs util/fetch/src/lib.rs util/reactor/src/lib.rs

coveralls · 2016-12-22T19:37:32Z

Changes Unknown when pulling afe25d7 on web-dapps3 into ** on master**.

jacogr · 2016-12-22T20:01:16Z

js/src/views/Web/web.css

+  height: 100%;
+  width: 100%;
+}
+.loading {


Would prefer a space between the blocks, doesn't cost anything but aids in readability. (Same for at the end of this block). -

.wrapper { ... } .loading { ... } .url { ... }

jacogr · 2016-12-22T20:01:20Z

js/src/views/Web/web.css

+  display: flex;
+}
+
+.url > input {


PostCSS nesting enabled, so would generally prefer -

.url { ... & > input { ... } }

jacogr · 2016-12-22T20:01:28Z

js/src/views/Web/web.js

+
+  handleUpdateUrl = (url) => {
+    try {
+      window.localStorage.setItem('parity-web-lastAddress', url);


Could use store and not need the catch block. In addition, would make the key a constant of the form

const LS_LAST_ADDRESS = '_parity::webLastAddress';

While we are still wildly inconsistent in naming of localStorage keys, @ngotchac started the _parity:: movement so it would make sense for new keys to conform.

jacogr · 2016-12-22T20:01:34Z

js/src/views/Web/web.js

+  lastAddress () {
+    let res = null;
+    try {
+      res = window.localStorage.getItem('parity-web-lastAddress');


As per above, would prefer the key a constant and using store (just a bit less boilerplate)

jacogr · 2016-12-22T20:01:38Z

js/src/views/Web/web.js

+    if (!token) {
+      return (
+        <div
+          className={ styles.wrapper }


Since we have a single attribute, could keep it on the preceding line.

jacogr · 2016-12-22T20:01:49Z

js/src/views/Web/web.js

+
+    return (
+      <div
+        className={ styles.wrapper }


Single attribute.

jacogr · 2016-12-22T20:01:52Z

js/src/views/Web/web.js

+  }
+}
+
+class AddressBar extends Component {


Would prefer the component at least in a separate file, just aids things not growing uncontrollably in a single source.

jacogr · 2016-12-22T20:01:54Z

js/src/views/Web/web.js

+}
+
+class AddressBar extends Component {
+


Unneeded blank line padding the class here. (Previous component didn't, which is preferred, although not always enforced.)

jacogr · 2016-12-22T20:01:58Z

js/src/views/Web/web.js

+
+  static propTypes = {
+    isLoading: PropTypes.bool.isRequired,
+    url: PropTypes.string.isRequired,


Alphabetical order so nobody needs to wonder is preferred here with properties, state, passing props, imports, etc. (Just making some sense of the chaos)

jacogr · 2016-12-22T20:02:02Z

js/src/views/Web/web.js

+    currentUrl: this.props.url
+  };
+
+  onUpdateUrl = (ev) => {


Could we move the event methods to beneath render? Lifecycle, render, events.

jacogr · 2016-12-22T20:02:04Z

js/src/views/Web/web.js

+  };
+
+  onKey = (ev) => {
+    const KEY_ESC = 27;


Should rather define these at global file-level

jacogr · 2016-12-22T20:03:06Z

js/src/views/Web/web.js

+          type='text'
+          value={ currentUrl }
+        />
+        <button


Would probably have used ui/Button here, but good as-is, we can tweak slightly as per your comments.

jacogr · 2016-12-22T20:03:09Z

js/src/views/Web/web.js

+      </div>
+    );
+  }
+


As per previous comment, unneeded blank line padding the class here.

coveralls · 2016-12-23T10:52:44Z

Changes Unknown when pulling fd62442 on web-dapps3 into ** on master**.

coveralls · 2016-12-23T12:59:47Z

Changes Unknown when pulling 9a58a77 on web-dapps3 into ** on master**.

coveralls · 2016-12-23T22:21:56Z

Changes Unknown when pulling f54c502 on web-dapps3 into ** on master**.

coveralls · 2016-12-27T09:05:38Z

Changes Unknown when pulling e00690d on web-dapps3 into ** on master**.

Tomasz Drwięga added 20 commits December 15, 2016 16:39

Dapps web

38fa18b

Conflicts: dapps/src/apps/fetcher.rs dapps/src/handlers/fetch.rs

Rewriting fetch

ac82701

Merge branch 'master' into web-dapps

f089d1f

Conflicts: Cargo.lock hash-fetch/Cargo.toml hash-fetch/src/client.rs rpc/Cargo.toml rpc/src/lib.rs rpc/src/v1/impls/parity_set.rs util/fetch/Cargo.toml util/https-fetch/Cargo.toml

Parity-wide fetch service

cd6d6df

Obey the limits and support cancellation.

7386b2b

Removing temporary files.

9b226ba

Actually use Fetch for dapps

7fd62dc

Merge branch 'master' into web-dapps

2bb9255

Re-implementing file fetching to avoid temporary files.

10e7a8b

Serde to 0.8.19

dc46d9e

Fixing content & dapps fetch

3e22b73

Serving web content and injecting scripts

cd32916

Don't wait for old content, start a new download

12c1c0b

Supporting timeouts and query

1079147

Simple GUI for the browser

9866d98

Proxy tokens validation

1c44541

Recovering from invalid web-based requests

a7379bb

Remember last visisted URL

45e0150

Removing unused variables

afe25d7

tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Dec 22, 2016

jacogr reviewed Dec 22, 2016

View reviewed changes

js/src/views/Web/web.js

return (

<div

className={ styles.wrapper }

Copy link

Contributor

jacogr Dec 22, 2016

Choose a reason for hiding this comment

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

Single attribute.

jacogr reviewed Dec 22, 2016

View reviewed changes

js/src/views/Web/web.js

};

onKey = (ev) => {

const KEY_ESC = 27;

Copy link

Contributor

jacogr Dec 22, 2016

Choose a reason for hiding this comment

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

Should rather define these at global file-level

jacogr reviewed Dec 22, 2016

View reviewed changes

js/src/views/Web/web.js

</div>

);

}

Copy link

Contributor

jacogr Dec 22, 2016

Choose a reason for hiding this comment

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

As per previous comment, unneeded blank line padding the class here.

Addressing review comments

fd62442

Tomasz Drwięga added 2 commits December 23, 2016 12:33

Setting default account in web3

2ef58fb

Adding WebBrowser dapp to the list

9a58a77

tomusdrw mentioned this pull request Dec 23, 2016

URL input on Applications tab #3957

Closed

Tomasz Drwięga added 2 commits December 23, 2016 18:00

Actually prune old entries when generating new token

f54c502

Merge branch 'master' into web-dapps3

2530fbf

Merge branch 'master' into web-dapps3

e00690d

gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 27, 2016

gavofyork merged commit c7c309d into master Dec 27, 2016

gavofyork deleted the web-dapps3 branch December 27, 2016 10:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Web Based Dapps #3956

Web Based Dapps #3956

tomusdrw commented Dec 22, 2016 •

edited

Loading

coveralls commented Dec 22, 2016

jacogr Dec 22, 2016

jacogr Dec 22, 2016

jacogr Dec 22, 2016 •

edited

Loading

jacogr Dec 22, 2016

jacogr Dec 22, 2016

jacogr Dec 22, 2016

jacogr Dec 22, 2016

jacogr Dec 22, 2016

jacogr Dec 22, 2016

jacogr Dec 22, 2016

jacogr Dec 22, 2016

jacogr Dec 22, 2016

jacogr Dec 22, 2016

coveralls commented Dec 23, 2016

coveralls commented Dec 23, 2016

coveralls commented Dec 23, 2016 •

edited

Loading

coveralls commented Dec 27, 2016

Web Based Dapps #3956

Web Based Dapps #3956

Conversation

tomusdrw commented Dec 22, 2016 • edited Loading

coveralls commented Dec 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacogr Dec 22, 2016 • edited Loading

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

coveralls commented Dec 23, 2016

coveralls commented Dec 23, 2016

coveralls commented Dec 23, 2016 • edited Loading

coveralls commented Dec 27, 2016

tomusdrw commented Dec 22, 2016 •

edited

Loading

jacogr Dec 22, 2016 •

edited

Loading

coveralls commented Dec 23, 2016 •

edited

Loading