brandly / Lax

IRC client built with Electron & React
https://github.com/brandly/Lax
MIT License
139 stars 26 forks source link

Channel joins with 0 members when not allowed to join #98

Open MitchTalmadge opened 4 years ago

MitchTalmadge commented 4 years ago

If you join a channel while not identified, or you need to be invited, such as joining #reddit on chat.freenode.net, the room will "open" with 0 people, and you can even "chat", but error messages will be sent to chat.freenode.net's main channel:

image

image

I don't know if this is a bug or not, I am pretty new to IRC. However, it is unintuitive. I would have expected the room to not open, with an error message dialog or something popping up. If nothing else, it would be nice to have a new message indicator on the chat.freenode.net label which would alert me to the server trying to tell me this information. On that note, it would help if chat.freenode.net was more apparently clickable -- I only learned that I could click on it by mistake.

brandly commented 4 years ago

I don't know if this is a bug or not, I am pretty new to IRC. However, it is unintuitive.

i agree. regardless of what's happening at the network level, the UI should do better at communicating what's going on and even offering a solution.

i believe this is where we receive that error, and there's zero sophistication in the error handling.

https://github.com/brandly/Lax/blob/b66850a8de4799d827d62fd0ee6af96c35341d46/src/js/actions/connections.js#L64-L66

i'm only touching the message on the event, but maybe there's a e.channel or something that would allow us to route it to a more appropriate place.

brandly commented 4 years ago

I would have expected the room to not open, with an error message dialog or something popping up.

i missed this line! depending on the sequence of events, maybe the room opens optimistically, and then the room itself shows an error indicator in the sidebar and a focused error message if that room is selected.

On that note, it would help if chat.freenode.net was more apparently clickable

this is true. a hover state would go a long way