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

Add presence support #32

Closed
wants to merge 5 commits into from

Conversation

sreecodeslayer
Copy link

There is a gotcha here:

Since there is no concept of user logins and sessions based on it, I felt its okay to mark a user as online once a message with a name is made. Presence would then track that particular user as opposed to tracking when the page loads.

Also, I've added a timestamp field to the Phoenix Presence tracker in the backend, but I chose to keep the UI simple.

Closes #14

@codecov
Copy link

codecov bot commented Oct 12, 2019

Codecov Report

Merging #32 into master will decrease coverage by 3.57%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #32      +/-   ##
==========================================
- Coverage     100%   96.42%   -3.58%     
==========================================
  Files           6        6              
  Lines          22       28       +6     
==========================================
+ Hits           22       27       +5     
- Misses          0        1       +1
Impacted Files Coverage Δ
lib/chat/application.ex 100% <ø> (ø) ⬆️
lib/chat_web/channels/room_channel.ex 93.75% <85.71%> (-6.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89d5d11...c6063c1. Read the comment docs.

Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

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

@sreecodeslayer We agree that Phoenix presence is a desirable feature to add. 🚀
However the purpose of this repository is to serve as a beginner tutorial for Phoenix.
As such we cannot accept a pull request without tests and documentation consistent with the existing example(s).
If you have time to add a couple of tests for the code, please do.
Thanks! 👍

@nelsonic
Copy link
Member

https://github.com/dwyl/phoenix-liveview-chat-example#14-presence

@nelsonic
Copy link
Member

nelsonic commented Feb 3, 2023

Given that there is quite a divergence from this PR to the main branch
and even larger once #152 (Phoenix v1.7 update) is merged.
I'm reluctantly closing this. 😞
Would love to have this contribution re-created off main with tests. 🤞

@nelsonic nelsonic closed this Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Could you add Phoenix Presence?
2 participants