[WIP] Update Dependencies and Small Improvements #605

hacdias · 2017-11-12T16:15:09Z

Update Dependencies and Other Improvements

File Manager/Preview Improvements (secondary)

daviddias · 2017-11-12T17:18:13Z

@hacdias Is this working for you locally? Maybe I started reviewing way too early :) Sorry about that!

hacdias · 2017-11-12T17:20:26Z

@diasdavid everything but file previews and objects' page.

hacdias · 2017-11-12T19:02:06Z

@diasdavid:

Is there any specific reason to use HashHistory (e.g.: address/#path) instead of BrowserHistory (e.g.: address/path)?
What's the difference between 'containers' and "legacy" pages?

daviddias · 2017-11-12T21:13:01Z

Ah, the error was on my end, I was using another branch of go-ipfs. Now I see it working:

Great work with the deps update, @hacdias! 👏🏽

re: Is there any specific reason to use HashHistory (e.g.: address/#path) instead of BrowserHistory (e.g.: address/path)?

When you use a # you are certain that information never goes to the server, it stays on the front end app.

What's the difference between 'containers' and "legacy" pages?

Containers come from Redux, are you familiar with it?

hacdias · 2017-11-12T21:16:55Z

I'm not, but I'll get familiar to it ;) I'm still updating object's page.

Right now, on master, previews are not working but they're working now. Although, I'm receiving a very strange error which doesn't have any side effect apparently.

Another thing I noticed are the 'Unmapped range' errors I'm getting on Connections page. I already know their source but I'm not understanding why they are happening here.

Do you have any thoughts on this?

daviddias · 2017-11-12T21:29:30Z

I'm also checking what's happening there, it seems that here:

https://github.com/ipfs/webui/blob/master/src/app/js/pages/connections.js#L56

We call getLocation (which is lookupPretty) with a PeerId and a multiaddr, but then it returns an error "Unmapped range" coming from https://github.com/ipfs/ipfs-geoip/blob/master/src/pretty.js#L15-L17 which only happens when we pass an empty array.

Now checking what goes inside the Peer :)

daviddias · 2017-11-12T21:34:01Z

It's actually coming from https://github.com/ipfs/ipfs-geoip/blob/master/src/lookup.js#L50. Bad error names, let me change that. Nevertheless, this means that geoip doesn't have info from the location of that IP, this should not error to the user and only show that location is Unknown. @hacdias think you can improve that part of the experience?

hacdias · 2017-11-12T21:35:52Z

Good to know you found the problem so fast 😮

daviddias · 2017-11-12T21:44:57Z

I've been around this codebase before 😉

geoip will need:

IPv6 support Add IPv6 support ipfs-shipyard/ipfs-geoip#60
Be tested with multiple multiaddrs now that we have things like Circuit Relay Create tests with a greater variety of multiaddrs ipfs-shipyard/ipfs-geoip#61

Until then, webui should not fail on the user, it should just show n/a

daviddias · 2017-11-12T21:45:45Z

@hacdias mind looking into #603 as well?

hacdias · 2017-11-12T21:51:39Z

@diasdavid sure thing.

hacdias · 2017-11-13T22:17:57Z

@diasdavid sorry, there are some things I'm needing help with:

~~This code is generating a Unexpected token A in JSON at position 0 which I think it's because 'name' will be 'undefined' at first. See more.~~ FIXED 😄
Later, I will probably need some help with the tests. I'm not used to write tests and I haven't already understood how they work (I'll try to dig more into them though).
~~I'm not fully understanding why the objects page is not working as expected. Although, the current version on master also produces some errors but works better.~~
~~What do you think is the most safe and simple URL scheme to use here:~~
- ~~#/files/?name=/path/to/dir~~
- #/files\/path/to/dir - This because hashes don't go into the server.

In the meanwhile, I'll make some improvements to the File Manager and the preview page and update this PR accordingly.

Edit (14th Nov, 10:32am): the object's page is now working better. The URLs' hash scheme is #/objects/ipfs/{hash}. Hashes with paths like #/objects/ipfs/QmYwAPJzv5CZsnA625s3Xf2nemtYgPpHdWEz79ojWnPbdG/readme don't work yet and the error Non-base58 character is produced from js-ipfs-api.

hacdias · 2017-11-16T16:38:08Z

@diasdavid take a look at the checkboxes above 😄 There are a lot of new ones!! I'd like to know if you could help me with the unchecked checkboxes from the first section ("Update Dependencies and Other Improvements").

About the File Manager: do you think a download button for files/folders would be great? What about a slightly improved design? One more question: we're using Bootstrap, right? 😃

Edit: what do you think about using Semantic UI instead of Bootstrap? They've got an official package for React and it doesn't seem that our current library (react-bootstrap) isn't getting much love recently!

daviddias · 2017-11-17T22:03:22Z

Still some deps to be updated

Here is a handy module to check things http://npmjs.com/npm-check-updates

hacdias · 2017-11-17T22:11:17Z

Yeah, I've been using that module. Thanks 😄

daviddias · 2017-11-17T22:12:38Z

Hi @hacdias! Just got to review your work, lots of progress here! Congratz 👏🏽👏🏽👏🏽

This PR is a bit exploding in size, let's focus on wrapping it up so that then multiple things can be tackled at the same time.

From what I understand, the things that are essential to merge this PR are:

Update tests and make sure CI is green
Update remaining dependencies
Fiz search by Object Path

Also, important question, are we able now to generate a build version to deploy with the next go-ipfs? The current webui go-ipfs ships is kind of out of date (//cc @whyrusleeping )

About the File Manager: do you think a download button for files/folders would be great? What about a slightly improved design? One more question: we're using Bootstrap, right? 😃

Edit: what do you think about using Semantic UI instead of Bootstrap? They've got an official package for React and it doesn't seem that our current library (react-bootstrap) isn't getting much love recently!

We picked Bootstrap at the time because of its community support, documentation and stability. I understand that the world has changed and that new UI frameworks can fit us better.

Before we jump to a decision, let's create a issue (you create it :)) in this repo to discuss what is the best and then we ask our community to help us decide given their experience.

Btw, since you are also a designed, how experienced are you with Invision and Sketch? Wanna work on the next UI together? I can create a team and invite you there.

hacdias · 2017-11-17T22:18:36Z

Thanks!

This PR is a bit exploding in size, let's focus on wrapping it up so that then multiple things can be tackled at the same time.

Sure, let's focus on its initial topic and update the dependencies. I'll need some help with the tests and the object by path. The remaining dependencies are updated 😄

We picked Bootstrap at the time because of its community support, documentation and stability. I understand that the world has changed and that new UI frameworks can fit us better.

Before we jump to a decision, let's create a issue (you create it :)) in this repo to discuss what is the best and then we ask our community to help us decide given their experience.

Btw, since you are also a designed, how experienced are you with Invision and Sketch? Wanna work on the next UI together? I can create a team and invite you there.

I'll open an issue then. I'm not experienced with Invision nor Sketch. Yeah, I'd like to work on the next UI 😮 It seems to be a challenging work to do and I could learn more about design I guess. 😄

hacdias · 2017-11-21T08:38:19Z

@diasdavid I think there is some kind of error on js-ipfs-api in config.replace which is returning File argument 'file' is required when we try to save the file. Could you check that, please?

daviddias · 2017-11-21T09:02:24Z

@hacdias I believe that was a bug introduced with ipfs-inactive/js-ipfs-http-client#629 which changed how files are sent to the daemon, wanna try figure that one out and added a test to make sure it works well? @pgte can help you by answering your questions :)

hacdias · 2017-11-21T09:05:42Z

I'll check that this evening. In the meanwhile, everything is greeen 😄

Edit: rebased everything atop of master. Also opened a PR on HenrikJoreteg/hjs-webpack#349 to support ES6.

Update dependencies Update dependencies Update dependencies Update Dependencies update dependencies! Fix Object Links Fix Globe Texture Update Babel to Version 7 Update React chore: remove lock files docs: give some love to the README Make views refresh the right way Make preview work Update GeoIP Fix "same-name directory" issue #603 Remove unused param Objects page already display the hash table Fix link to Objects page Fix nameon preview Remove unnecessary elements Fix object's view Update File Explorer and File Preview Styles Sweet file explorer! Use urls like /objects/ipfs/hash instead of /objects\/ipfs\hash avoid double slash Add upload button to action bar Remove safe-buffer and use built-ins Update js-ipfs-api Rudimentary Rename (not working) Make Move work :D F2, esc and del keys Rename on Action Bar CTRL+Click, Multiple files shift-CLICK to select range of files update dependencies Remove fetch objects by path remove useless comments update deps fix 'npm run build' remove coment and add pr link Use fork of hjs-webpack update some tests fix config.spec.js update circle.yml

daviddias · 2017-11-21T10:03:51Z

👏🏽👏🏽👏🏽👏🏽👏🏽👏🏽 Excellent work @hacdias

This means that we finally can ship new WebUIs to go-ipfs //cc @whyrusleeping

hacdias · 2017-11-21T10:04:51Z

Thanks @diasdavid!

daviddias

Some tidbits, otherwise it LGTM :)

I would appreciate some review from the folks that have worked on the webui project before.

daviddias · 2017-11-21T10:05:22Z

package.json

-    "webpack": "^3.0.0"
+    "style-loader": "^0.19.0",
+    "url-loader": "^0.6.2",
+    "webpack": "^3.8.1"
  },
  "pre-commit": [
    "lint"


Sooo greeeen! :D

Am I supposed to change anything in here? 😮

just happy :)

daviddias · 2017-11-21T10:07:59Z

src/app/js/containers/files-explorer.js

+      break
+    }
+
+    this.selectAllBetween(fi, la)


@hacdias can we convert this loop into something more JavaScript'y/Functional? Check out all the beautiful array methods https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array and try to use variable names that try to be more explicit (it is ok to use full words in JS land)

daviddias · 2017-11-21T10:08:40Z

src/app/js/containers/files-explorer.js

+    for (let i = fi; i <= la; i++) {
+      const filePath = join(root, list[i].Name)
+      select(filePath)
+    }


Yeah, let's make this more JavaScripty, I have no idea what is fi and la

ghost assigned hacdias Nov 12, 2017

ghost added the status/in-progress In progress label Nov 12, 2017

ghost assigned daviddias Nov 12, 2017

daviddias self-requested a review November 12, 2017 21:45

hacdias changed the title ~~[WIP] Update Dependencies~~ [WIP] Update Dependencies and Small Improvements Nov 13, 2017

daviddias mentioned this pull request Nov 17, 2017

Currently doesn't work for production build #603

Closed

hacdias force-pushed the update-dependencies branch from 6386bf9 to e46508d Compare November 21, 2017 09:19

daviddias mentioned this pull request Nov 21, 2017

UglifyJS ipfs-inactive/js-ipfs-http-client#611

Closed

daviddias requested review from victorb and dignifiedquire November 21, 2017 10:04

daviddias requested review from olizilla, alanshaw and lidel November 21, 2017 10:04

daviddias requested changes Nov 21, 2017

View reviewed changes

Make some loops more javascripty

d24e747

daviddias approved these changes Nov 21, 2017

View reviewed changes

update dep

f42933b

daviddias mentioned this pull request Nov 22, 2017

config.replace stopped working ipfs-inactive/js-ipfs-http-client#633

Closed

daviddias merged commit 2c2a305 into master Nov 22, 2017

daviddias deleted the update-dependencies branch November 22, 2017 10:29

ghost removed the status/in-progress In progress label Nov 22, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Update Dependencies and Small Improvements #605

[WIP] Update Dependencies and Small Improvements #605

hacdias commented Nov 12, 2017 •

edited

Loading

daviddias commented Nov 12, 2017

hacdias commented Nov 12, 2017

hacdias commented Nov 12, 2017

daviddias commented Nov 12, 2017

hacdias commented Nov 12, 2017

daviddias commented Nov 12, 2017

daviddias commented Nov 12, 2017 •

edited

Loading

hacdias commented Nov 12, 2017

daviddias commented Nov 12, 2017

daviddias commented Nov 12, 2017

hacdias commented Nov 12, 2017

hacdias commented Nov 13, 2017 •

edited

Loading

hacdias commented Nov 16, 2017 •

edited

Loading

daviddias commented Nov 17, 2017

hacdias commented Nov 17, 2017

daviddias commented Nov 17, 2017 •

edited

Loading

hacdias commented Nov 17, 2017

hacdias commented Nov 21, 2017

daviddias commented Nov 21, 2017

hacdias commented Nov 21, 2017 •

edited

Loading

daviddias commented Nov 21, 2017

hacdias commented Nov 21, 2017

daviddias left a comment

daviddias Nov 21, 2017

hacdias Nov 21, 2017

daviddias Nov 21, 2017

daviddias Nov 21, 2017

daviddias Nov 21, 2017

[WIP] Update Dependencies and Small Improvements #605

[WIP] Update Dependencies and Small Improvements #605

Conversation

hacdias commented Nov 12, 2017 • edited Loading

Update Dependencies and Other Improvements

File Manager/Preview Improvements (secondary)

daviddias commented Nov 12, 2017

hacdias commented Nov 12, 2017

hacdias commented Nov 12, 2017

daviddias commented Nov 12, 2017

hacdias commented Nov 12, 2017

daviddias commented Nov 12, 2017

daviddias commented Nov 12, 2017 • edited Loading

hacdias commented Nov 12, 2017

daviddias commented Nov 12, 2017

daviddias commented Nov 12, 2017

hacdias commented Nov 12, 2017

hacdias commented Nov 13, 2017 • edited Loading

hacdias commented Nov 16, 2017 • edited Loading

daviddias commented Nov 17, 2017

hacdias commented Nov 17, 2017

daviddias commented Nov 17, 2017 • edited Loading

hacdias commented Nov 17, 2017

hacdias commented Nov 21, 2017

daviddias commented Nov 21, 2017

hacdias commented Nov 21, 2017 • edited Loading

daviddias commented Nov 21, 2017

👏🏽👏🏽👏🏽👏🏽👏🏽👏🏽 Excellent work @hacdias

hacdias commented Nov 21, 2017

daviddias left a comment

Choose a reason for hiding this comment

daviddias Nov 21, 2017

Choose a reason for hiding this comment

hacdias Nov 21, 2017

Choose a reason for hiding this comment

daviddias Nov 21, 2017

Choose a reason for hiding this comment

daviddias Nov 21, 2017

Choose a reason for hiding this comment

daviddias Nov 21, 2017

Choose a reason for hiding this comment

hacdias commented Nov 12, 2017 •

edited

Loading

daviddias commented Nov 12, 2017 •

edited

Loading

hacdias commented Nov 13, 2017 •

edited

Loading

hacdias commented Nov 16, 2017 •

edited

Loading

daviddias commented Nov 17, 2017 •

edited

Loading

hacdias commented Nov 21, 2017 •

edited

Loading