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

feat: show all protocols in connectivity #1167

Merged
merged 1 commit into from
Sep 19, 2019
Merged

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Sep 15, 2019

Fixes #1141. Shows all protocols in the connectivity column. This is an alternative for #1166. Only one must be merged.

image

License: MIT
Signed-off-by: Henrique Dias [email protected]

License: MIT
Signed-off-by: Henrique Dias <[email protected]>
Copy link
Member

@lidel lidel 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 this one better (unlike #1166 this PR does not hardcode anything and is more future proof).
ipfs・p2p-circuit will get a lot more useful when #1169 lands Peers screen will have all the info user may want 👍

@lidel
Copy link
Member

lidel commented Sep 19, 2019

FYI noticed interesting edge case with js-ipfs in Brave:
when ws-star multiaddrs are present, protocol list does not fit and get trimmed:

2019-09-19--15-37-02

Personally, I think it is the right way to display this type of edge case. There is no length or nesting limit, so truncating is expected. Ellipsis hints uses to hover and when #1169 lands the full multiaddr will be displayed which is what I would expect from this type of UI.

In other words, no blocker here, ok to merge.

@hacdias hacdias merged commit 7d20a80 into master Sep 19, 2019
@hacdias hacdias deleted the feat/show-all-protos branch September 19, 2019 15:49
@hacdias
Copy link
Member Author

hacdias commented Sep 19, 2019

Thanks for the review @lidel. I also prefer this one. Hence, I am merging it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Peers: WebSockets transport displayed as TCP
2 participants