nextcloud / talk-android

📱😀 Video & audio calls through Nextcloud on Android
Other
538 stars 243 forks source link

scroll to bottom button in chat #2470

Closed mahibi closed 1 year ago

mahibi commented 2 years ago

chats should have a scroll to bottom button in the lower right corner

rapterjet2004 commented 1 year ago

Hey, I'd like to try my hand on this issue. Can you assign me to it?

mahibi commented 1 year ago

Hi @rapterjet2004 that's great, thanks for your interest to help :+1: you should have received an invitation to the nextcloud android team. This way you can create branches and pull requests without the need to fork the repo.

Just let us know if you need any help!

rapterjet2004 commented 1 year ago

Hey, I cloned the repo and got the project set up, but I can't login to the app in my emulator without a server address. Sorry if I'm asking an obvious question. Is there some kind of test development server I can use? Or do I have to set up my own? I don't recall seeing anything about it in the setup or contribution docs. Thanks for inviting me to the team btw!

mahibi commented 1 year ago

The easiest way for you will be: just get an account on our nextcloud instance so you can also benefit from community chats and so on. Just send me a message to marcel . hibbe at nextcloud . com and tell me the email address that you want to be invited with.

One other solution would be to use docker with https://github.com/juliushaertl/nextcloud-docker-dev to setup an instance on your machine, but depending on your setup this might need some fiddling to connect the emulator.

mahibi commented 1 year ago

for this feature it might be helpful to have a look at https://github.com/nextcloud/talk-android/pull/1588 which should be similar technical wise...

rapterjet2004 commented 1 year ago

Hey Marcel, I'm getting a render issue when previewing controller_chat.xml. I can view the file normally using the layout validation tab, but for some reason I can't preview the file, unless I comment out <com.stfalcon.chatkit.messages.MessagesList/> which is really weird. I'm on Android Studio Electric Eel | 2022.1.1, and clicking on the rendering issue gives me this stack trace

java.lang.reflect.InvocationTargetException at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ... Caused by: java.lang.IllegalArgumentException: You can't set adapter to MessagesList. Use #setAdapter(MessagesListAdapter) instead. at com.stfalcon.chatkit.messages.MessagesList.setAdapter(MessagesList.java:55) ... 31 more

Not sure what's wrong with the adapter, if anything. I tried cleaning and rebuilding my project and even deleted the entire thing and recloned it, but it still go away.

Here's it with the tag commented out

image

Here's it with the tag uncommented

image

And heres the stack trace

image
AndyScherzinger commented 1 year ago

Hi @rapterjet2004, you unfortunately have to ignore that for the time being since it comes from the lib we use ChatKit which provides this UI element, yet enforces a different adapter method to be used by throwing an exception o the default method here which is due to this element extending RecyclerView, so there is currently nothing we can do about that.

See: https://github.com/stfalcon-studio/ChatKit/blob/d10cfe3393a9d6ce150e817b22b019dcd17c55fa/chatkit/src/main/java/com/stfalcon/chatkit/messages/MessagesList.java#L50-L57

rapterjet2004 commented 1 year ago

Hey guys, so I copied the colors and the chevon down svg from the website dashboard and created a mockup for the buttons. Is the size and layout alright?

image
mahibi commented 1 year ago

looks good in my opinion :+1:

i would make this an image button with "baseline_keyboard_arrow_down_24" which can be chosen in android studio as vector asset: https://developer.android.com/studio/write/vector-asset-studio

Then apply android:background="@drawable/shape_oval" on it to make it round.

Regarding the colors and sizes/margins @AndyScherzinger might have an opinion?

If you feel comfortable with it, feel free to already push it and create a pull request (can be "draft"), so we could exchange opinions in the PR overview or in the code-view..

mahibi commented 1 year ago

Thinking about the feature, it would make sense to combine it UI-wise with the "unread messages" feature. grafik

So whenever there are new messages, there could be a the "scroll to bottom" button with an additional indicator (something like notification badge or shown number to unread messages) instead of the current "unread messages" bubble.

But this can also be a followup in the future. Feel free to give it a try or to do the plain scroll to bottom button.

AndyScherzinger commented 1 year ago

The unread message bubble and combining it UI-wise is a discussion for later I think, also Jan is on vacation for 3 weeks now and also the scroll-to-bottom "mini-fab" is build like that on Whatsapp, so I think that is fine, same for Talk Web btw.

As for the colors, feel free to leave them like that, when there is a PR I can run locally I can better check the colors with the accessibility scanner to ensure that it is fine contrast-wise light and dark theme. The mockups look fine asking my eyes, but I'll check with a proper scanner tool then.

So I'd say let's go with the mock-up! Nice work @rapterjet2004 - love that fact you made mock-ups for discussing the specifics! 💙

AndyScherzinger commented 1 year ago

The icon is fine for now, while I would build a down-pointing "double chevron" icon later, since the single chevron icon usually points towards an "expand" action. So nothing to worry about right now for this PR.

AndyScherzinger commented 1 year ago

@rapterjet2004 don't worry too much about the colors, we will need to add an additional implementation for that in our common-library later since our apps do fully dynamic coloring programmatically based on the color we receive from the server.