Skip to content
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

Improved Automatic Mode and Options UX #277

Merged
merged 8 commits into from
Sep 10, 2017
Merged

Improved Automatic Mode and Options UX #277

merged 8 commits into from
Sep 10, 2017

Conversation

lidel
Copy link
Member

@lidel lidel commented Sep 4, 2017

@lidel lidel added kind/enhancement A net-new feature or improvement to an existing feature UX labels Sep 4, 2017
@lidel lidel self-assigned this Sep 4, 2017
Node status panel now uses CSS flexbox, which produces
the same look under Firefox and Chrome.
- flex-based layout that looks okay under both Firefox and Chrome
- added normalization of the list of public gateways
  - duplicate spaces, semicolons and commas are replaced with new line
    when displayed in GUI (one host per line)
  - when stored as a property, line endings are replaced with single space
This should address UX concerns raised in #274
Text is hardcoded for now, we will move it to locale files
in separate PR.
@lidel
Copy link
Member Author

lidel commented Sep 7, 2017

I've added a quick description for every option on the Preferences screen and made it look kinda decent under both Firefox and Chrome.

Please let me know if something sounds odd or could be replaced with better label/description.
Github Review/Comment is preferred. Reward is one virtual cake.

2017-09-07-204307_983x1097_scrot

(cc @mateon1 from #274)

<label for="publicGateways">
<dl>
<dt>Public Gateways</dt>
<dd>list of hostnames known for exposing IPFS resources under <code>/ipfs/</code> path</dd>

Choose a reason for hiding this comment

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

Recommend rewording:
<dd>List of hostnames known to expose IPFS under <code>/ipfs/</code></dd>

Copy link
Member Author

Choose a reason for hiding this comment

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

🍰

<label for="ipfsApiPollMs">
<dl>
<dt>Status Poll Interval</dt>
<dd>how often peer count is refreshed (in miliseconds)</dd>
Copy link
Member Author

Choose a reason for hiding this comment

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

from AndyS2:

Time interval after which peer count is refreshed (in milliseconds)

Copy link

Choose a reason for hiding this comment

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

I believe "How often peer count is refreshed" is fine, use simpler words whenever it makes sense.

<label for="customGatewayUrl">
<dl>
<dt>Custom Gateway</dt>
<dd>URL address of preferred HTTP2IPFS&nbsp;Gateway</dd>
Copy link
Member Author

Choose a reason for hiding this comment

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

from AndyS2:

dunno if 'URL address' is correct. it's either URL or web address

Copy link

Choose a reason for hiding this comment

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

Agreed with Andy, it should probably be "URL"

<label for="publicGateways">
<dl>
<dt>Public Gateways</dt>
<dd>list of hostnames known for exposing IPFS resources under <code>/ipfs/</code> path</dd>
Copy link
Member Author

@lidel lidel Sep 7, 2017

Choose a reason for hiding this comment

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

from AndyS2 (cake was provided via IRC):

starting everything small looks a bit weird. How about 'A list of hostnames ...' and changing everything like that?

@lidel lidel requested a review from daviddias September 7, 2017 19:49
@lidel lidel changed the title Automatic Mode: toggle redirect only when API is going online/offline Improved Automatic Mode and Options UX Sep 7, 2017
Copy link

@mateon1 mateon1 left a comment

Choose a reason for hiding this comment

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

I left a few notes, otherwise looks good 👍

@@ -1,5 +1,4 @@
<html>

<!DOCTYPE html>
Copy link

Choose a reason for hiding this comment

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

Adding a doctype is good, but now the <html> tag was removed! (And github doesn't escape < html > in review comments... Oops)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think its plain only in review comments.
Syntax is highlighted correctly when full file is displayed.

ps. 🍰

<label for="ipfsApiPollMs">
<dl>
<dt>Status Poll Interval</dt>
<dd>how often peer count is refreshed (in miliseconds)</dd>
Copy link

Choose a reason for hiding this comment

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

I believe "How often peer count is refreshed" is fine, use simpler words whenever it makes sense.

<label for="customGatewayUrl">
<dl>
<dt>Custom Gateway</dt>
<dd>URL address of preferred HTTP2IPFS&nbsp;Gateway</dd>
Copy link

Choose a reason for hiding this comment

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

Agreed with Andy, it should probably be "URL"

<label for="useCustomGateway">
<dl>
<dt>Use Custom Gateway</dt>
<dd>replace requests to <em>Public</em> gateway with <em>Custom</em> one</dd>
Copy link

Choose a reason for hiding this comment

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

"Redirect requests made to Public gateways to the Custom gateway"

@@ -1,5 +1,4 @@
<html>

<!DOCTYPE html>
Copy link

Choose a reason for hiding this comment

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

Same in this file, the tag was removed

@lidel lidel merged commit 1343241 into master Sep 10, 2017
@lidel lidel deleted the 274-redirect-toggle branch September 10, 2017 18:11
@lidel
Copy link
Member Author

lidel commented Sep 10, 2017

Big thanks and 🍰 for everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants