-
Notifications
You must be signed in to change notification settings - Fork 324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Nice "Share files via IPFS" with file drop. #335
Conversation
Looks pretty good! -- the peers number is misleading though, we share this data with many more nodes, it's just that they're not currently connected to us. About the |
@lgierth thanks! I think having a couple of stats helps keep it interesting. I looked at what station shows https://github.com/ipfs-shipyard/station#a-menubar-ipfs-application-to-get-you-on-the-distributed-web I've not got much in my repo, so it's probably not a good test of the perf on stats requests. Would it be reasonable to campaign for stats to be cached inside ipfs core, so that we can visualise them in UIs without worrying about it slowing down with repo size / usage? We currently show the same peer count on the badge on the browser add on button. Have you got any thoughts on what could make it less misleading. I'm ok with showing the user how many nodes they can see right now, even if it's not the whole story... my intention was for it to be a simple way to make new users feel the p2p joy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks super nice! Added 2 small 'buts' :)
add-on/src/popup/quick-upload.js
Outdated
})} | ||
<div class="pl3"> | ||
<h1 class="f2 fw5 ma0">${browser.i18n.getMessage('panel_quickUpload')}</h1> | ||
<p class="f3 fw2 lh-copy ma0 light-gray">Currently sharing <strong class="fw6">${filesCount}</strong> files with <strong strong class="fw6">${peerCount}</strong> IPFS peers</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we reword it a bit to indicate that we talk about 'active' connections? I am not a native speaker but maybe something like:
Currently sharing 3389 files (and|while) connected to 1038 IPFS peers.
add-on/src/popup/quick-upload.js
Outdated
<div class='dt dim' style='padding-left: 100px; height: 300px'> | ||
<div class='dtc v-mid'> | ||
<span class="f3 link dim br1 ph4 pv3 dib white" style="background: #6ACAD1"> | ||
Pick a file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel adding i18 keys (locales) should be a part of this PR – otherwise we may miss it and make translators super sad 😿
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Sorry, that was lazy of me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thinking out loud I think I generally hold off on i18n until I've got some general approval on a new design.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that you mentioned it, I got your point: it may be counter-productive to translate too early (eg. we may change/remove labels and translation work is lost). Especially now that we have multiple people working on multiple things.
Perhaps we could make the process more efficient by agreeing that it is okay to commit english inline in places that can change before stable release, and instead add "localization" step to the "non-beta release" checklist?
Text is extracted to It was useful to pull the english text into the |
Add a nice upload page layout and add repo and peer stats.
TODO: