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: direct messages #120

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

feat: direct messages #120

wants to merge 21 commits into from

Conversation

dozyio
Copy link
Contributor

@dozyio dozyio commented Apr 28, 2024

  • Adds direct messaging
  • Adds toast for errors
  • local dev server over ssl
  • Autodialer for gossipsub peers
  • Pinger to drop dead connections
  • Opinionated eslint & prettier
  • Replaced download/blockies with react-18-blockies
  • Fixed chat height to match viewport
  • Moved some functionality to libs
  • Unread message count
  • Timestamps
  • Spinner while message sends
  • disable file uploads button when direct message - ideally move file uploads to Helia in a separate PR
  • DM page for mobile
  • back to public chat link on DMs page when on mobile

Copy link

vercel bot commented Apr 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
universal-connectivity ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 29, 2024 2:34pm

@dozyio dozyio changed the title feat: direct messages (wip) feat: direct messages Apr 28, 2024
@dozyio dozyio marked this pull request as ready for review April 28, 2024 14:35
@dozyio dozyio requested a review from a team as a code owner April 28, 2024 14:35
}),
dht: kadDHT({
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the reason for enabling the DHT in the browser again?

I added delegated routing to avoid running a DHT client in the browser, but if there's a good reason, we can enable.

Also see #113 for more background on why this app relies on the IPFS DHT.

console.log('undiscovered peer', evt.detail.from.toString())

if (libp2p.getConnections().length < AutoDialerMaxConnections) {
await libp2p.dial(evt.detail.from)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you tried this out and can you tell whether this works for two browsers behind NAT?

@2color
Copy link
Collaborator

2color commented Apr 29, 2024

Thanks for submitting this PR. I skimmed through the PR and looked at the preview and it looks like you've cleaned up and refactored a lot which is great.

Before giving this a more thorough review, some initial feedback:

  • I'm reluctant to add custom protocols to this app, due to the complexity and maintenance overhead it adds. However, I see the value in direct and signed messaging.
  • I'm currently working on getting browser-to-browser connections to work properly out-of-the-box and well documented (with some ambient peer discovery mechanism). This is something that I'd like to get working and documented ideally before merging this. I saw that you attempt this in the AutoDialer component, but I am pretty sure that for browser-to-browser to work, the browser needs a circuit relay reservation and a multiaddr dialable from the browser that ends with p2p-circuit/webrtc/p2p/peer_id, e.g. /ip4/147.28.186.157/udp/9090/webrtc-direct/certhash/CERT_HASH/p2p/12D3KooWGahRw3ZnM4gAyd9FK75v4Bp5keFYTvkcAwhpEm28wbV3/p2p-circuit/webrtc/p2p/OTHER_BROWSER_PEER_ID.

@2color
Copy link
Collaborator

2color commented May 23, 2024

Any thoughts @dozyio. Really appreciate this PR!

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.

None yet

2 participants