Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

api/http: add HTML Progress Bar #1859

Merged
merged 15 commits into from
Oct 8, 2019

Conversation

significance
Copy link
Member

@significance significance commented Oct 3, 2019

This is an initial version of a HTML progress bar.

I would like to use this PR to get something in principle agreed, and begin then to iterate to improve and fine tune user experience, styling and so on, as well as:

  • capture any UI/UX ideas and especially first impressions
  • check in a variety of OS/Browser environments to ensure compatibility

I've used the current go templating framework and inline svgs, base64 fonts and various other quirks are intended to enable us to easily package everything in the Swarm binary and stay within the established code review procedures so as not to add overhead to this initial development.

In the future and particularly as the scope of the javascript application increases, it may be worth considering moving to a separate repo to take advantage of a modern build chain and perhaps some well chosen plugins such as a single page app framework.

for info, here are are the html, js, css and assets i'm working with https://github.com/significance/swarm-progress-bar

@significance significance requested a review from acud October 3, 2019 16:41
@acud acud changed the title Adds HTML Progress Bar api/http: add HTML Progress Bar Oct 4, 2019
Copy link
Member

@janos janos left a comment

Choose a reason for hiding this comment

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

I like the new design and functionality, but I think that few things can be improved.

I gave some comments also as a user feedback which are completely subjective and debatable.

One functionality that may be useful is setting the default entry on upload. After the upload the hash is presented and if the same hash is used in Browse form, the error Not found: could not find resource '' is returned which is rather cryptic as the manifest does not have the default entry. It is required to append the filename to the link in order to get the file only by hash.

Another thing is that for some reason I expected the browse to be default view resulting me clicking in the file input and opening the browse file dialog, instead to type the address. This may be just a problem with my expectations.

<style>
{{ template "css" . }}
</style>
<title>Swarm Gateway</title>
Copy link
Member

Choose a reason for hiding this comment

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

I do not think that this page is meant only for gateways, but for any swarm node. Maybe to remove the work Gateway? Also from the svg logo?

Copy link
Member Author

@significance significance Oct 5, 2019

Choose a reason for hiding this comment

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

I think this would be worth some discussion - for me 'gateway' related to 'http gateway' so this is semantically accurate, but I perhaps I'm wrong about this?

If so - would it perhaps be good to have descriptive term to replace it rather than leaving the header as only 'Swarm'?

api/http/templates.go Outdated Show resolved Hide resolved
api/http/templates.go Outdated Show resolved Hide resolved
api/http/templates.go Show resolved Hide resolved
api/http/templates.go Outdated Show resolved Hide resolved
api/http/templates.go Show resolved Hide resolved
api/http/templates.go Outdated Show resolved Hide resolved
api/http/templates.go Outdated Show resolved Hide resolved
@significance
Copy link
Member Author

significance commented Oct 5, 2019

@acud @janos thanks for reviews, glad you like it!

Above commits should deal with most of the issues.

Regards directory upload and setting default-path, these require a bit of work to implement, if you agree, perhaps we can go with this as an MVP and add these functionality later (soon)?

I'll investigate in the meantime.

@acud
Copy link
Member

acud commented Oct 7, 2019

@significance please fix the travis builds

@janos
Copy link
Member

janos commented Oct 7, 2019

Thanks @significance for addressing the comments. There is a failing test api.TestGet that needs to be adjusted.

The question about word Gateway in title remains and it is good to hear other opinions, as well.

@acud
Copy link
Member

acud commented Oct 7, 2019

I agree with @janos that we shouldn't have gateway in the title

@acud acud merged commit a2c765f into ethersphere:master Oct 8, 2019
@skylenet skylenet added this to the 0.5.3 milestone Oct 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants