search

relaycc / receiver

Relay Receiver is a React library that makes it easy to add Web3 messaging to your website.
http://www.demo.relay.cc
MIT License
38 stars 2 forks source link

Making sure mobile scrolls to the bottom of the conversation after se… #93

Closed BrianS111 closed 1 year ago

BrianS111 commented 1 year ago

Closes https://github.com/relaycc/receiver/issues/91

BrianS111 commented 1 year ago

This should be done shortly

BrianS111 commented 1 year ago

@killthebuddh4 I'm taking a quick break, but shouldn't we have the message list and the message input in the same component? I'm having trouble passing refs to them through their parent. I'll push some code in a bit once it's not a complete mess from my attempts... But if they were one, it seems like this would be a very easy issue to complete

killthebuddh4 commented 1 year ago

@killthebuddh4 I'm taking a quick break, but shouldn't we have the message list and the message input in the same component? I'm having trouble passing refs to them through their parent. I'll push some code in a bit once it's not a complete mess from my attempts... But if they were one, it seems like this would be a very easy issue to complete

If you mean they should be in the same containing component, then yes, but I think they are already are ( in the PeerAddress component). If you mean that the JSX for both the list and the input should be defined together inside a single, then no because the list and the input are not always together in the app. If passing refs between the two components is the way to get scrolling to work, and if there's no way to do it without merging the two components, then that's what we have to do. That said, it's not clear to me why/how both those things would be necessary, I'll know more once I can take a look at the code.

BrianS111 commented 1 year ago

Yeah, I was having trouble setting what I need in PeerAddress and getting it to the necessary components. useRef, two versions of react DOM type stuff. I think I'm going to try a different way though

BrianS111 commented 1 year ago

I pushed a working version, it's not 100% done, but I'll get to the details tomorrow. Let me know what you think about the direction I decided to go in and if you want me to set it up in a different way.

killthebuddh4 commented 1 year ago

I pushed a working version, it's not 100% done, but I'll get to the details tomorrow. Let me know what you think about the direction I decided to go in and if you want me to set it up in a different way.

I think scrolling to a ref when some event happens is a perfectly reasonable approach. When I attempted to fix this bug, that's how I did it. I wasn't able to get it to work and my thought at the time was that maybe something additional is needed, but that could totally not be the case.

In terms of the event that triggers the scroll, my hunch is that the event will need to be triggered by an incoming message rather than user input and that we won't need to pass props between the input and the list. A working example, regardless of how it's done, is definitely the first step here. We can worry about "the right way" later.

BrianS111 commented 1 year ago

Im working on testing on actual mobile phones instead of dev tools and getting screen recordings now. Also just finished getting typescript under control

BrianS111 commented 1 year ago

Right now it's working perfectly except for new message bubbles in certain situations where it's barely off. "It's working 95% of the time 100% of the time" Should be an easy fix as well as solving other issues on IOS

BrianS111 commented 1 year ago

Taking a couple hour break from this issue to knock out some easier ones marked as ready. The last 5% of this one is more difficult than anticipated. I'll be back at it in a bit

killthebuddh4 commented 1 year ago

Taking a couple hour break from this issue to knock out some easier ones marked as ready. The last 5% of this one is more difficult than anticipated. I'll be back at it in a bit

If you could provide a breakdown of what's working and what's not working (the last 5%) and also push the current state, we could decide whether or not to continue spending time on this.

BrianS111 commented 1 year ago

New Message bubbles don't scroll all the way down to show the first new message in some situations. The thing is, I brought the I phone from the office home to test on and I can't even replicate the first error on the live site. I can't replicate it with the current code (from this push) on the simulator. Current push adds the functionality that scrolls down after sending also.

BrianS111 commented 1 year ago

If anyone can QA this better than I can and let me know if this works it would be much appreciated. Maybe we push this and debug from there? Unless you see any glaring holes in this code or something that would cause an isssue

BrianS111 commented 1 year ago

This push is the best state to go with ATM

BrianS111 commented 1 year ago

And by this push I mean the one that has already been pushed. Currently I was getting fancy and it's a mess