search

adventures-in / chat_app

A group effort to build a chat app and learn as we go.
18 stars 0 forks source link

stream already been listened to when user clicked on their second conversation #204

Closed gaslitbytech closed 4 years ago

gaslitbytech commented 4 years ago

This is a impulsive fix to a issue that I found while creating a new feature. This was that I was not able to click another conversation once I clicked into the first. I instinctively started to think it was the _controller being reused for all conversations. I almost didn't get this far as I accidently did the fix initially within _database.getMessagesStream(conversationId).listen((messagesList) { inner function? And When I tried to return the `_controllers[conversationId].stream I'd get a null exception as the changes within this inner function I suppose it was that it was not in scope. Anyway this seems to work and fixes the bug. I'll add a issue to track this bug now.

nickmeinhold commented 4 years ago

Hey mate, I attempted to start a discussion in #205 - I think it's worth discussing our approach to managing the streams - keeping a stream controller for each conversation is certainly one option - my instinct is that there are better options but I would be happy to have my mind changed. My suggestion would be that we take the time to talk it through as it will be educational and we can explore advantages and disadvantages of each approach. If that's frustrating please do let me know so we can try and find a way forward that works for us both. Thanks! :-)

gaslitbytech commented 4 years ago

My suggestion would be that we take the time to talk it through

Happy with that though I this talk shouldn't delay this PR as this issue makes it hard / impossible to use this app.

nickmeinhold commented 4 years ago

Fair enough, I’ll revert the PR - it shouldn’t have gone up without tests I suppose, I created issues to track making tests but that was probably a bad idea.

gaslitbytech commented 4 years ago

Don't think we should revert. I like the idea of separate issues. Though yes need to act on them this week.