feat: add hover and clusters to peers map #1444

rafaelramalho19 · 2020-04-18T01:29:33Z

Notes

I wasn't sure what information to place in the popover, so I just used the country and id for now. If you find any more relevant information I should place, please say so 🙏
I couldn't find a way to properly make a reducer work with createAsyncResourceBundle, the documentation seemed a bit lacking and I had some really hard time trying to figure out what happened. After reading through the source code I found out that createAsyncResourceBundle actually creates a reducer function to handle all the async mechanism mutations, so I extended it. You can see this in src/bundles/peers.js#13
If someone could help me find a better way, I would be very much appreciated 😄
Added some transitions to smoothen the interaction with several popovers, as you can see here:

Questions

I believe that most of the times the peers will be too close to each other (at least on the map scale) and hovering through them can be weird/glitchy. Should we change the map scale or add buttons to zoom?
Should we scroll the list to the hoverable peer when it's out of view?
Should we show the popover when we also hover an element from the list?

PS: Sorry for the long merge request description 😅

jessicaschilling · 2020-04-18T03:03:08Z

This is awesome! My thoughts on your questions but @hacdias and @lidel should weigh in too 😊

Zoom buttons would be really cool
So would scrolling
If we have zoom buttons, and we scroll through the list to a region that's out of view on the map, the popover might not be in view. That might be OK, but soliciting others' opinions? (Flying through the map feels too unwieldy?)

hacdias · 2020-04-18T07:12:21Z

I love this and I agree with @jessicaschilling about scrolling.

lidel · 2020-04-20T13:39:00Z

@rafaelramalho19 I really like the idea of map popover!

May be a bit more useful if popover for a standalone peer includes its full multiaddr (ok to keep the shortened PeerID).. and maybe latency too?

In other words, instead of "12D3..y8PK"
something like: "/ip4/13.40.98/tcp/4001/p2p/12D3..y8PK (130ms)"

FYSA multiaddr prefix is already displayed on hover at connection column, if you want to reuse the data source.

Answering questions:

(1) Should we change the map scale or add buttons to zoom?

Adding zoom would not fully solve the problem, because no matter how much you zoom, you will always end up with situation when there is more than one person from a big city, and they all are assigned the same coordinate on the map (geoip resolution we use is limited to the "city" level).

I don't think we should add zoom in this PR (it could be something for the future / separate PR). Instead, we should make the map smarter if there is more than one peer in specific area (eg. within the size of the dot itself).

Loose idea: merge those dots into a single (bigger?) one and show a popover with a peer count for the area. Could still have a list of merged peers below the count.

This would make it easy to hover mouse on parts of the USA, see how many of my peers are from NYC, Tokio. Right now it gets messy when there are many peers (some people tweak settings and run webui with 5000 peers or more!), no way to tell how much bigger US is than EU, especially when viewport is small (could be 10x more peers, but both look the same):

As for (2) and (3):
Should we scroll the list to the hoverable peer when it's out of view?
Should we show the popover when we also hover an element from the list?

I think those are great improvements for UX, but should be implemented together: if one works, user will expect it working the other way around.

(I suspect it may be easier to implement without zoom controls, as there is no need to account for edge cases when zoomed in and hovering something outside of map viewport)

ps. long PR descriptions with visuals like the one you made here are great, they ensure everyone is on the same page, keep doing it! ❤️

rafaelramalho19 · 2020-04-20T13:50:55Z

Good suggestion @lidel, I will merge the dots into a bigger one and share the result here.

I'm actually having problems with paning and zooming with d3 + our react setup, so your suggestion actually helps me a lot 😄

jessicaschilling · 2020-04-20T19:23:36Z

+1 to everything @lidel said. Thanks both for thinking through this in such detail 😊

src/peers/WorldMap/WorldMap.js

jessicaschilling · 2020-04-21T15:00:58Z

Following up on the bi-weekly call to add a reference to the NY Times coronavirus map illustrating relative size of dots with numbers attached to them. This is overkill for our case, but could be useful inspiration: https://www.nytimes.com/interactive/2020/us/coronavirus-us-cases.html

rafaelramalho19 · 2020-04-21T17:40:24Z

New update:

Grouped the peers in a range of 8 dots together, so they look like this now:
Added labels to indicate how many peers there are per dot type.
The hovering effect now contains a +X label indicating that there are more than 5 peers in a circle
The hover effect updates all peers in a circle:

@jessicaschilling @hacdias @lidel what you think of this approach?

jessicaschilling · 2020-04-21T18:01:45Z

This is so cool 😊

A few cosmetic suggestions, assuming @lidel @hacdias are OK with this approach:

Could the key please go underneath the map as a whole? Otherwise it looks like we have a bunch of peers in Antarctica. We should probably precede the row with the word "Key:", too.
In the hover, the latency is a little higher up than the peerID. Also, can the latency figure please be in gray, to differentiate it from the rest of the peerID?
In the hover, if the "+X" label is to indicate the total number of peers "contained in a circle", we should be more explicit that's the total. Assuming that the top 5 we are seeing in the hover are the top 5 in the list as sorted by the user's sort choice (latency in your video), one suggestion might be:
- In the area where you currently have "+X", instead a header in the same teal uppercase as the table sort headers: "Top X by {sort choice}"
- At the bottom of the list of 5, justified right, if there are more than 5: "Plus XXX more peers"

Thanks!

rafaelramalho19 · 2020-04-22T10:44:40Z

src/bundles/peer-locations.js


-      const unique = new Set(allCoord.map(JSON.stringify))
-      return Array.from(unique).map(JSON.parse)
+      return peers.reduce((previous, { peerId, coordinates }) => {


Note to self: Missing tests once UI and UX is reviewed

rafaelramalho19 · 2020-04-22T12:45:54Z

This is so cool 😊

A few cosmetic suggestions, assuming @lidel @hacdias are OK with this approach:

Could the key please go underneath the map as a whole? Otherwise it looks like we have a bunch of peers in Antarctica. We should probably precede the row with the word "Key:", too.

In the hover, the latency is a little higher up than the peerID. Also, can the latency figure please be in gray, to differentiate it from the rest of the peerID?

In the hover, if the "+X" label is to indicate the total number of peers "contained in a circle", we should be more explicit that's the total. Assuming that the top 5 we are seeing in the hover are the top 5 in the list as sorted by the user's sort choice (latency in your video), one suggestion might be:

In the area where you currently have "+X", instead a header in the same teal uppercase as the table sort headers: "Top X by {sort choice}"

At the bottom of the list of 5, justified right, if there are more than 5: "Plus XXX more peers"

Thanks!

Thanks for the suggestions Jessica,

Relative to the last point, I didn't sort the elements in the hover with the same logic as in the table because that added an extra layer of overhead and complexion.
However, If it's something you really see improving the experience, I can do it 😄 .

Now, about the other points you referred, could you please revise them?

Key under the map:

Latency grayed & Plus XXX more peers:

jessicaschilling · 2020-04-22T15:23:53Z

This is great! I'd like to get @lidel's blessing but I think this is enough to send out into the world. Ideally we should sort the contents of the hover boxes, but I'll let @lidel weigh in on whether that's worth the effort at this stage.

lidel

I really, really like it @rafaelramalho19! 👌 😍

Took it for a spin and found some small bugs. Also, wrote some thoughts how we can optimize clustering logic for regular users who will have under 300 peers (see below).

UI bugs / nits

Contents of the hover box will look better if we sort them by multiaddr
(those are just strings, just sort them in lexicographical order, that is all)
For some reason I get horizontal scrollbar in Firefox, may be related to the other issue (3):
I believe we have a regression in responsiveness – when I resize window the map does not scale, it starts overlaping the menu:

src/bundles/peer-locations.js

rafaelramalho19 · 2020-04-23T13:46:38Z

I really, really like it @rafaelramalho19! 👌 😍

Took it for a spin and found some small bugs. Also, wrote some thoughts how we can optimize clustering logic for regular users who will have under 300 peers (see below).

For some reason I get horizontal scrollbar in Firefox, may be related to the other issue (3):

I believe we have a regression in responsiveness – when I resize window the map does not scale, it starts overlaping the menu:

Those were some issues I found at the start of the development, so I was thinking about opening a new merge request after this one to fix them. But since you already referred it, I'll put it in this MR 😄

Co-Authored-By: Marcin Rataj <lidel@lidel.org>

…o19/ipfs-webui into feat/add-hover-to-peers-map

rafaelramalho19 · 2020-04-23T17:42:23Z

Thanks for the suggestions @lidel, here's how it looks so far:

Also added a transition when new peers appear to smoothen the experience 🚀

I would love to work again on this later and improve responsiveness (mobile needs a lot of love in the map)

lidel

Thanks! Made another pass:

Some peers have no latency info, in those cases we should either hide (ms) or show (???ms):
- There should be some whitespace between the flag and the address, ideally the same amount as in the beer list below the map.
- Would it be possible to reuse multiaddr formatter from the Status screen? It has grayed out protocol sections making it super easy to look at:
For some time I had this weird bug where menu randomly stopped working. Finally figured it out – svg is getting over the menu, hijacking clicks in Firefox. Perhaps tweaking z-index will be enough to fix this?
Smaller suggestions below

src/bundles/peer-locations.js

lidel · 2020-04-24T01:08:48Z

src/peers/WorldMap/WorldMap.js

+      <div className='relative flex justify-end pt2 pb4'>
+        <div className='f6 p2 no-select flex items-center'>
+          <span className='f6 charcoal-muted pr3'>{t('index')}: </span>
+          <i className='mapDotExplanation mr1' style={{ width: getDotsSize(1) * 2, height: getDotsSize(1) * 2, backgroundColor: getDotsColor(1) }}></i>1-10 {t('peers')}
+          <i className='mapDotExplanation ml3 mr1' style={{ width: getDotsSize(50) * 2, height: getDotsSize(50) * 2, backgroundColor: getDotsColor(50) }}></i> 10-100 {t('peers')}
+          <i className='mapDotExplanation ml3 mr1' style={{ width: getDotsSize(110) * 2, height: getDotsSize(110) * 2, backgroundColor: getDotsColor(110) }}></i>100+ {t('peers')}


i18n nit: having "Peers" multiple times may look bad in languages where the world is really long.

I believe it will be safer to simplify this to something like:

This will also enable us to add one more level, if we want.
(eg. 1-10, 10-50, 50-100, 100+)

public/locales/en/peers.json

rafaelramalho19 · 2020-04-24T11:34:27Z

Thanks! Made another pass:

Some peers have no latency info, in those cases we should either hide (ms) or show (???ms):

There should be some whitespace between the flag and the address, ideally the same amount as in the beer list below the map.

Would it be possible to reuse multiaddr formatter from the Status screen? It has grayed out protocol sections making it super easy to look at:

For some time I had this weird bug where menu randomly stopped working. Finally figured it out – svg is getting over the menu, hijacking clicks in Firefox. Perhaps tweaking z-index will be enough to fix this?

Smaller suggestions below

Thanks for the review @lidel!

The flag actually has empty space in MacOS, so that's why I never saw that happen 😢
I've added padding to the flag so it doesn't happen, here are examples:

MacOS flag:

Windows flag:

I was able to fix the scrollbar issue, but I needed to get a bit creative and actually get the popover to align down instead of up. If you see this as a UX problem (CC @jessicaschilling), I'll try a more overhead solution (passing the popover as a portal element, and setting the top and left according to the body/html position, while calculating the size of the svg and subtracting offsets)

Nice suggestion on the Status component, didn't see it 👍

jessicaschilling · 2020-04-24T16:08:01Z

I don't think the popover facing down is a big problem. If there are a ton of peers in Patagonia, it might overlap with the top of the main peer table, but that feels like a bit of an edge case for this release.

hacdias

On the UX/UI side, I love it and I approve it. I didn't check the whole code though. But, the parts I checked, looked good for me.

lidel · 2020-04-27T14:57:36Z

@rafaelramalho19 mind resolving the conflict? (rebase is also fine)

lidel · 2020-04-27T15:00:08Z

@hugomrdias @alanshaw does any of you have bandwidth to take a look at React-specific parts and give @rafaelramalho19 some feedback?

src/bundles/peer-locations.js

…t/add-hover-to-peers-map

rafaelramalho19 · 2020-04-28T11:15:13Z

@lidel Everything's ready for merge now, just waiting on your final approval 🎉

lidel

Fantastic contribution @rafaelramalho19, let's ship it!

rafaelramalho19 added 2 commits April 16, 2020 23:13

feat: add hover to peers map

1487011

feat: add hover effect to peers map

2029e14

jessicaschilling requested review from hacdias and lidel April 18, 2020 02:55

feat: add more info to the peers popover

938ee84

lidel requested changes Apr 21, 2020

View reviewed changes

src/peers/WorldMap/WorldMap.js Outdated Show resolved Hide resolved

feat: add multiple sizes and colors to the peers map

4e3f3b3

rafaelramalho19 requested a review from lidel April 21, 2020 17:31

rafaelramalho19 commented Apr 22, 2020

View reviewed changes

rafaelramalho19 added 2 commits April 22, 2020 11:51

chore. move the peers map explanation to the bottom

107ddb1

chore: change latency in hover to gray

7c90c97

chore: update more peers copy

c4ccfd3

lidel requested changes Apr 23, 2020

View reviewed changes

src/bundles/peer-locations.js Outdated Show resolved Hide resolved

rafaelramalho19 and others added 3 commits April 23, 2020 14:47

chore: update coordinates radius

fbb9380

Co-Authored-By: Marcin Rataj <lidel@lidel.org>

chore: refactor peer locations tests to use modular mocking

0610143

Merge branch 'feat/add-hover-to-peers-map' of github.com:rafaelramalh…

ac62049

…o19/ipfs-webui into feat/add-hover-to-peers-map

feat: add transition to the peers map dots

787d0d7

lidel requested changes Apr 24, 2020

View reviewed changes

rafaelramalho19 requested a review from lidel April 24, 2020 11:53

chore: fix peers map overflow

47bc9ae

jessicaschilling approved these changes Apr 24, 2020

View reviewed changes

hacdias approved these changes Apr 25, 2020

View reviewed changes

lidel mentioned this pull request Apr 27, 2020

Release 0.11.0 ipfs/ipfs-desktop#1415

Closed

22 tasks

lidel requested review from alanshaw and hugomrdias April 27, 2020 14:57

hugomrdias suggested changes Apr 27, 2020

View reviewed changes

src/bundles/peer-locations.js Show resolved Hide resolved

src/bundles/peer-locations.js Show resolved Hide resolved

style: Plus → +

e7626cb

hugomrdias approved these changes Apr 27, 2020

View reviewed changes

Merge branch 'master' of github.com:ipfs-shipyard/ipfs-webui into fea…

7f5a550

…t/add-hover-to-peers-map

lidel approved these changes Apr 28, 2020

View reviewed changes

lidel changed the title ~~Feat/add hover to peers map~~ feat: add hover and clusters to peers map Apr 28, 2020

lidel merged commit 8111aa4 into ipfs:master Apr 28, 2020

jessicaschilling mentioned this pull request Apr 28, 2020

Show tooltip on plotted dots on peers page #819

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add hover and clusters to peers map #1444

feat: add hover and clusters to peers map #1444

rafaelramalho19 commented Apr 18, 2020

jessicaschilling commented Apr 18, 2020 •

edited

Loading

hacdias commented Apr 18, 2020

lidel commented Apr 20, 2020 •

edited

Loading

rafaelramalho19 commented Apr 20, 2020

jessicaschilling commented Apr 20, 2020

jessicaschilling commented Apr 21, 2020

rafaelramalho19 commented Apr 21, 2020

jessicaschilling commented Apr 21, 2020

rafaelramalho19 Apr 22, 2020

rafaelramalho19 commented Apr 22, 2020

jessicaschilling commented Apr 22, 2020

lidel left a comment

rafaelramalho19 commented Apr 23, 2020

rafaelramalho19 commented Apr 23, 2020

lidel left a comment •

edited

Loading

lidel Apr 24, 2020 •

edited

Loading

rafaelramalho19 commented Apr 24, 2020

jessicaschilling commented Apr 24, 2020

hacdias left a comment

lidel commented Apr 27, 2020

lidel commented Apr 27, 2020

rafaelramalho19 commented Apr 28, 2020

lidel left a comment •

edited

Loading

feat: add hover and clusters to peers map #1444

feat: add hover and clusters to peers map #1444

Conversation

rafaelramalho19 commented Apr 18, 2020

Notes

Questions

jessicaschilling commented Apr 18, 2020 • edited Loading

hacdias commented Apr 18, 2020

lidel commented Apr 20, 2020 • edited Loading

rafaelramalho19 commented Apr 20, 2020

jessicaschilling commented Apr 20, 2020

jessicaschilling commented Apr 21, 2020

rafaelramalho19 commented Apr 21, 2020

jessicaschilling commented Apr 21, 2020

rafaelramalho19 Apr 22, 2020

Choose a reason for hiding this comment

rafaelramalho19 commented Apr 22, 2020

jessicaschilling commented Apr 22, 2020

lidel left a comment

Choose a reason for hiding this comment

UI bugs / nits

rafaelramalho19 commented Apr 23, 2020

rafaelramalho19 commented Apr 23, 2020

lidel left a comment • edited Loading

Choose a reason for hiding this comment

lidel Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

rafaelramalho19 commented Apr 24, 2020

jessicaschilling commented Apr 24, 2020

hacdias left a comment

Choose a reason for hiding this comment

lidel commented Apr 27, 2020

lidel commented Apr 27, 2020

rafaelramalho19 commented Apr 28, 2020

lidel left a comment • edited Loading

Choose a reason for hiding this comment

jessicaschilling commented Apr 18, 2020 •

edited

Loading

lidel commented Apr 20, 2020 •

edited

Loading

lidel left a comment •

edited

Loading

lidel Apr 24, 2020 •

edited

Loading

lidel left a comment •

edited

Loading