revoltchat / rvmob

React Native Revolt client. Community-led project.
https://rvlt.gg/rvmob
GNU Affero General Public License v3.0
115 stars 17 forks source link

feat: quality-of-life UI improvements and misc changes #27

Closed alexispurslane closed 10 months ago

alexispurslane commented 10 months ago

Please make sure to check the following tasks before opening and submitting a PR

Features

Misc Changes

alexispurslane commented 10 months ago

For my next PR I'm going to try to make the process of restoring your chat location faster, by loading it with the UI instead of loading the UI and then changing the state again, I'm going to try to fix the message grouping by time, and I'm going to try to make messages just stay in frame without needing to auto scroll every time

alexispurslane commented 10 months ago

Question: many of the components in this application currently are far too gigantic and mixed together in functionality to be manageable, it makes it very difficult to find where anything is done or make changes. Would you be open to accepting a pool request from me refactoring a lot of the gigantic 500 line components into subcomponents? Most of the sub components probably won't be reused in any way but I still think breaking it up and separating the concerns a little bit will make it more readable and easy to change.

Rexogamer commented 10 months ago

I have been moving to refactor/split things more - I would prefer this be done somewhat gradually, but I would be open to PRs which help with this effort.

Now would also be a good time to mention that I've been moving to a new message view using a flat list instead (currently accessible via the Debug page on the DM list) - as such, while these changes are fine (incoming review comments aside), I wouldn't necessarily spend too much time on the current one at this point.

alexispurslane commented 10 months ago

I have been moving to refactor/split things more - I would prefer this be done somewhat gradually, but I would be open to PRs which help with this effort.

Sounds good, I'll see what I can do!

Now would also be a good time to mention that I've been moving to a new message view using a flat list instead (currently accessible via the Debug page on the DM list) - as such, while these changes are fine (incoming review comments aside), I wouldn't necessarily spend too much time on the current one at this point.

Got it. I actually almost switched to flatlist in my local codebase lol. I'll check it out and see how it feels.

alexispurslane commented 10 months ago

ah well, looks like I'm not working on it today. Gradle is shitting itself even though I've changed nothing about my build environment except pulling the new changes from upstream, so I can't get it to even build the app and i really don't feel like dealing with its bullshit (I spent six hours trying to get it to work the first time)

Rexogamer commented 10 months ago

No rush - I'll test these changes locally and post some review comments soon, but you can take your time ^^

alexispurslane commented 10 months ago

Okay in a few days when I'm feeling better I'll get on these changes. I also have some improvements I want to try to rhe last channel code

alexispurslane commented 10 months ago

@Rexogamer I figured out what was wrong with atLatestMessages, the base message objects returned by the API call to get messages from a channel were just being wrapped in order to add grouping information so I had to do .message._id. So I'm adding in the atLatestMessages logic to make scrolling down work. But I've noticed that since the new commits, bottomOfPage no longer works, which means that and a couple other scrolling related things (including my logic to make sure that if the user has manually scrolled up, that when a new message is added they don't get scrolled down, which is something you noticed) don't work now, so now I've got to track down that issue.

alexispurslane commented 10 months ago

I've discovered that the scrolling forward functionality, now that I've gotten it to be called properly.... doesn't work. It produces this weird unholy mess where you have two interleaved copies of exactly the same messages when you scroll forward. It's weird. Trying to track down this bug now. Then I'll try to get on a reliable method of checking if you're at the first messages in a channel, because as it turns out we need this for scrolling up to work right at all for short channels.

Rexogamer commented 10 months ago

I'm tempted to suggest splitting this PR up a bit - most of this (the last channel/server feature, the font size setting, the setting remarks, the Gradle tweaks, the design updates and the AMOLED re-enablement) could all be merged now, while the stuff that seems to be needing more investigation (I'm assuming just the scrolling changes?) could be split off into its own PR to not hold the rest of it up.

alexispurslane commented 10 months ago

I'm tempted to suggest splitting this PR up a bit - most of this (the last channel/server feature, the font size setting, the setting remarks, the Gradle tweaks, the design updates and the AMOLED re-enablement) could all be merged now, while the stuff that seems to be needing more investigation (I'm assuming just the scrolling changes?) could be split off into its own PR to not hold the rest of it up.

That's a good idea, it would just be a lot of work to do so, and I'm starting to despair of this entire project because I've spent about four hours today trying to get/keep the react native build system even working at all, and the app (with no changes on my end) has decided to completely stop running at all just recently, and nothing I can do seems to change it. Plus, I pulled your upstream changes, and there's several unhandled promise errors every few seconds and I can't get a stack trace on them that even ends anywhere in the code at all. I just really hate react native tbh. I'm usually far more productive than this but there's no room for me to be productive when I have to spend all day dealing with flimsy webdev bullshit (sorry for the cursing)

Rexogamer commented 10 months ago

It's fine - I can create a new branch and revert any changes I would rather not merge, and you'll still be listed as the commit author. I'm slightly confused, however - I haven't seen any unusual errors on my end? I agree, however - React Native can be a massive pain at times