User feed page refactor

hjiangsu commented 3 months ago

Pull Request Description

This is the second attempt at refactoring the user feed page. This time, I've decided to limit down the scope of implementation to be concentrated on the feed (rather than both the feed and account pages). Because of this, there are some files which I've renamed (as they'll be deprecated in the future) to allow the account page to use the previous logic.

Caveats:

Comment actions are not implemented yet. This means that swipe actions, or any action on a comment will not be triggered at this time.
Some FAB actions don't make sense when in the user page/comments tab (create post, subscriptions (as a new page is pushed), dismiss read)
When triggering the user sidebar, it looks a bit weird since the posts/comments section hides itself (otherwise, the sidebar would show under the posts/comments tab bar)

Issue Being Fixed

Issue Number: N/A

Screenshots / Recordings

Checklist

[ ] Did you update CHANGELOG.md?
[ ] Did you use localized strings where applicable?
[ ] Did you add semanticLabels where applicable for accessibility?

micahmo commented 3 months ago

Good luck! 😊😊

hjiangsu commented 3 months ago

@micahmo I think this is functional enough for you to try it out! I sure I missed something during my implementation so let me know if you notice any weird behaviours. I mentioned some caveats I found while testing this myself (will most likely attempt to fix the first and second bullet points in separate PRs later)

Also, no need to do any code reviews or anything since there's been a lot of changes 😅

Fmstrat commented 3 months ago

Just because of time I wish I had but don't, I would highly recommend merging the read-on-scroll branch before this one, it looks like there will be conflicts here, and you'll be much more suited to understanding them than me.

hjiangsu commented 3 months ago

I agree! I'll be merging in all other PRs before I merge this one in to minimize as many conflicts as possible from your end 😄

This is still in draft too since I feel like there's more testing to be done

micahmo commented 3 months ago

First of all, nice work tackling one of the big refactoring tasks we know we have to do! Per your recommendation, I did not review the code :blush: I just played around with things a bit.

I did notice a couple funny things about the header and sidebar. It looks like the header is now word wrapping, which looks funny when there's no natural breakpoint. It also looks like there's some render overflow on the sidebar, despite there being no content down there. (Note that this is only on my user profile page, but I think that does include your sidebar changes.)

The toggle button hiding is a bit funny, but at least it's animated, so we can live with that!

I was wondering how comment sorting works since some of the modes don't apply (at least to comments within a post).

That's all for now, nothing too crazy!

hjiangsu commented 3 months ago

It looks like the header is now word wrapping, which looks funny when there's no natural breakpoint.

Interesting! I copied the behaviour of the community header, so that might have something to do with it. Maybe I can change it here so that it scales the text down based on the length of the name.

It also looks like there's some render overflow on the sidebar, despite there being no content down there.

Hmm, I'll take a look at that!

The toggle button hiding is a bit funny, but at least it's animated, so we can live with that!

Yeah, I wasn't sure of a good way to do it without overcomplicating the logic. This is because the tabs are not technically a part of the header, so when the sidebar shows up, it shows up under the tab bar (which I think looks more awkward to say the least) If you have any suggestions, let me know!

I was wondering how comment sorting works since some of the modes don't apply (at least to comments within a post).

The endpoint that we use to retrieve the user posts/comments are the same, and it seems like it takes in the sorting of the post 😅 I think the Lemmy backend handles this some way but I'm not too sure myself

hjiangsu commented 3 months ago

Alright, I've updated this a bit to fix some of the issues:

The username should now fit in one line. This does mean that the font size is now variable, but hopefully it shouldn't be too big of a deal!
Added back the user share option (missed it when I was re-introducing the user app bar) and also moved some options to the overflow menu to (to match more with the community app bar) - more specifically, moved the refresh/share into the overflow menu. Let me know what you think!

It also looks like there's some render overflow on the sidebar, despite there being no content down there.

This might be due to the SizedBox that is at the end of the sidebar to allow it to scroll up more. I don't think this affects functionality per-say, so I left it as is.

micahmo commented 3 months ago

The username should now fit in one line. This does mean that the font size is now variable, but hopefully it shouldn't be too big of a deal!

Oh yeah, that looks awesome! I wonder about the full name on the line below. In my screenshot, it was able to break at the bullet, but what if the name were so long that it had to break in the middle of the text? Should we just overflow will ellipses there, or apply the same fix with the font scaling? Also this would be sweet in the community header as well, but that can be later. 😊

Otherwise I think this is good to go!

hjiangsu commented 3 months ago

I wonder about the full name on the line below. In my screenshot, it was able to break at the bullet, but what if the name were so long that it had to break in the middle of the text? Should we just overflow will ellipses there, or apply the same fix with the font scaling?

I was a bit torn on this. I tried to make the full name fit in one line, but that made it difficult to read for some long usernames. Using ellipses makes it so that we can't fully read the name which is also something I'd like to avoid. I think we can leave it as is for now and see if this is an issue?

Otherwise I think this is good to go!

Nice! I'll wait until I can merge in some other PRs first (e.g., mark as read on scroll) and then merge this in!

micahmo commented 3 months ago

I think we can leave it as is for now and see if this is an issue?

Sounds good! By the way, I compared against the current behavior. It looks like the short username is truncated with a fade out and the full name is truncated with ellipses. Wrapping is fine for now; maybe we can think of something more elegant in the future. 😊

hjiangsu commented 2 months ago

I'll go ahead and merge this in! Let me know if you find any additional issues while running with this change 😄

micahmo commented 2 months ago

Woohoo!

micahmo commented 2 months ago

Just FYI, it looks like the full username wrapping is not quite right with a really long name (ThunderDevTesting • sh.itjust.works)

micahmo commented 2 months ago

Just FYI, in addition to the above, I found another different behavior on the current user page vs. other pages. The former has no shadow over the sidebar while the latter does.

Header	Header

thunder-app / thunder