Refactor: feed page

[hjiangsu]

Pull Request Description

This is a complete refactor of the feed page (also known as community_page). The purpose of this is to:

• Reorganize widget structure to be smaller and more modular
• Allow the use of slivers rather than fixed app bars to allow for flexible app bars (and also make way for hiding app bar)

This is currently WIP and is not fully up to parity with the current features. Below will be a list of the features which are currently known which need to be completed

• Open sidebar when on the main page. When on a new page, be able to go back to the previous page
• Ability to open sort bottom sheet and switch between sort types
• Ability to scroll back to top when tapping on the feed icon on the bottom nav bar
• Ability to see community header, and trigger the community information
• Ability to perform actions when swiping
• Ability to perform actions on button presses
• Ability to perform all actions when long pressing a post

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?

[hjiangsu]

There's still quite a bit to be done here, but @micahmo, could you possibly do some testing on this when you have the chance? It doesn't have to be now, but I just wanted to give a heads up!

I'm sure theres tons of things I'm still missing here to match parity with the current features so it would be good to have someone else test this out and provide some feedback on what's still missing :D

Things that I know of at this time which are still missing:

• Add back taglines into the feed
• Mark post as read when tapping on it
• Add back refresh button on app bar
• Creating a new post FAB action is broken
• Duplicate posts are not filtered out
• Sidebar indicators are missing (to let you know which feed you're currently on)
• Move loading spinner to the center when loading posts
• Fix issues with pull-down refresh indicator
• Fix issues with app bar title not showing up properly
• Feed not refreshing after switching accounts
• Make back button platform specific
• Modal barrier should be lighter in colour when in light mode
• Make default back icon respect Android styled buttons and only use iOS back icon when on iOS
• Remove general feed title slivers
• Increase contrast with modal barrier for light mode feed FAB
• Improve community sidebar to match parity with current community sidebar
• Make communities respect the default sort type
• Add back subscribe action on communities

I think theres some Android specific features that are not yet re-implemented as well (back button related actions)

My goal is to hopefully have the refactor completed by the end of this week if possible. There's still a lot of cleaning up to do after I re-implement all the missing features

[micahmo]

@hjiangsu Sure thing, I can start daily driving this branch. I'm happy to live with the quirks for now, and I'll try to only mention things that you haven't already covered.

[micahmo]

Here are my first findings in just a few minutes. Overall looks pretty solid and on the right track. As you said, little things here and there need cleaning up.

• The loading spinner is at the top rather than centered. (Not a big deal either way, but I liked centered.)
• The pull-down refresh indicator comes from a weird place (slightly above the bottom of the app bar).

qemu-system-x86_64_uHD4Y7iquq.mp4

• The app bar title is not visible until the feed is scrolled.

qemu-system-x86_64_GsqjYzmBmt.mp4

• The back button in the feed back looks more like the iOS style than the Android style.

• The feed doesn't appear to refresh after switching accounts/instances.
• In light mode, the color of the background is wrong when expanding the FAB (should get lighter, not darker)

That's all for now! I think live testing this will be much more fruitful than a code review at this point, so I'm happy to keep running for a while. Also I know this will probably be a bit painful, but good for the overall architecture in the long run!

[hjiangsu]

Thanks for catching those! I'll update those points to my original comment so that I can keep track of them all

That's all for now! I think live testing this will be much more fruitful than a code review at this point, so I'm happy to keep running for a while. Also I know this will probably be a bit painful, but good for the overall architecture in the long run!

Yeah, I don't want anyone to endure this code review (even myself) 😅

[hjiangsu]

@micahmo I've fixed a lot of the issues that I previously mentioned and that you mentioned! There's still a couple left to do but feel free to do some more testing on it and let me know if you notice anything else

[micahmo]

Sounds good, will give it another run tonight!

Regarding this logic...

Platform.isAndroid ? const Icon(Icons.arrow_back_rounded) : const Icon(Icons.arrow_back_ios_new_rounded)

Do you think you could flip it and only use arrow_back_ios_new_rounded on iOS? I only say that because I think arrow_back_rounded is the default (for example, if you don't override the leading icon in the AppBar) on all other platforms (i.e., web, desktop).

---

EDIT

Just gave this another test spin. Looking amazing!! Definitely addressed most of the things I noticed. In addition to my comment above about the back icon selection, here are some more thoughts. (Apologies for piling these on, I'm just writing down what I see! 😆)

• The pull-down refresh icon is now appearing a bit lower than I think it should (in the middle of the community header).

qemu-system-x86_64_VlshtcYNwx.mp4

• On the main feed (non-community view), there seems to be a bunch of wasted space before scrolling. Maybe we don't need the cool sliver transition and can just populate the app bar directly? Or is there something else we could put in that area until scrolled (like the instance name)? Just a thought.

• I don't see taglines, although you checked that task.

• The background color when expanding the FAB is not quite enough. It's still hard to read the options. In #706, I had used theme.colorScheme.background.withOpacity(0.95).

• The sidebar has some quirks.

  • It fades out at the top and bottom, which looks a little funny.

  • I can't tap outside to close it (only swipe).

  • Scrolling down within the sidebar refreshes the feed (and fails).

    qemu-system-x86_64_JUjAnlhNkd.mp4

  • There's an unexpected delay after closing the sidebar before the dimmed overlay is removed (and there's no animation).

    qemu-system-x86_64_TdyZqzGm1A.mp4

  • Also the appearance is now very different from the user page.

  • The buttons (new post, subscribe) don't seem to work.

• Communities do not respect the default sort mode.

• Communities don't have subscribe button in app bar.

Ok that's all for now! I know it seems like a lot, but the amount of work you've put into this refactor is amazing!!

[hjiangsu]

Do you think you could flip it and only use arrow_back_ios_new_rounded on iOS?

Yeah I can do that!

The pull-down refresh icon is now appearing a bit lower than I think it should (in the middle of the community header)

Dang, okay, I'll have to adjust the values again. The positioning of it is controlled by an integer because the refresh indicator is within a CustomScrollView so its just a matter of tweaking it so that it aligns properly

On the main feed (non-community view), there seems to be a bunch of wasted space before scrolling. Maybe we don't need the cool sliver transition and can just populate the app bar directly?

Yeah, we can do that (or add some more info) like you mentioned! I was just doing some playing around with slivers while doing the refactor 😅

I don't see taglines, although you checked that task.

Hmm, I tested that with reddthat.com and it was working there, but I can double check

The background color when expanding the FAB is not quite enough.

I'll increase it a bit more!

The sidebar has some quirks.

Yeah... theres still stuff to be done there that I forgot about. I'll continue to fix up the stuff related to the sidebars. The thing is right now, the user page and the feed page are separate pages (user page still follows the original methods) so a lot of things are broken there

I'll go ahead and update the previous comment to add these points in! Thanks :D

[hjiangsu]

This is with 0.95 opacity - do you think this is too much?

[micahmo]

do you think this is too much

No, I think it's perfect! For comparison, here's Google Cal.

[hjiangsu]

I think I would have the community info 'blink' in, rather than slide in?

Hmm, do you have an example of what you mean by 'blink'?

Also I kinda liked the main FAB action turning into a close button, but also don't feel too too strongly about the proposed new behavior.

I think this can be debated on too! If most people like the existing implementation, then I can revert it back (this was an unintentional change to begin with, so I don't have too much of a preference myself)

Should tapping the community banner have an inkwell effect?

I think this makes sense! I'll add that in as well

[CTalvio]

My main concern with the full width sidebar, is how it would work on larger devices. We already have dual column feeds, but a lot of other things really aren't very usable on a large device. Even with dual-columns, stuff like comment threads are still full-width and quite awkward. We should look to applications like gmail which make effective use of multiple panes of application content.

Aside from that, I'm quite attached to the overlay look, that's what "sidebar" had always meant to me over the years. I want to be able to open it for any given community from the future pull-up subscription list I want to make (this functionality would make no sense with the current list in the left drawer), or pull it out on the create post screen to check the rules of the community I'm posting to.

My original design for it was made with the intention of being able to summon it in all kinds of contexts.

I would want to keep the main action becoming a close button for the expanded FAB.

[hjiangsu]

We already have dual column feeds, but a lot of other things really aren't very usable on a large device.

Yeah, I agree that we need to rethink how the overall app feels and looks on larger devices. However, to do this, we need to do some major refactoring (starting out with this feed page) so that it's less dependent on other widgets/logic. Once this is done for the major views/pages (feed, post, comment, etc), then we can start applying these views in a more modular way to adapt to larger devices without completely breaking everything.

Aside from that, I'm quite attached to the overlay look, that's what "sidebar" had always meant to me over the years.

I see what the concern is here! Maybe I'll try out a few more tricks/tweaking around to see if I can make it better without adding too much complexity to the code.

I would want to keep the main action becoming a close button for the expanded FAB.

Seems like the majority here goes towards the close button so I'll make that change!

[machinaeZER0]

Hmm, do you have an example of what you mean by 'blink'?

It may have been a bug, but in your video the feed was disappearing and then the community panel was sliding into view, which may have just been a bug? In the current release the sidebar animation pops out over the feed, which continues to be drawn.

It sounds like we're probably moving back to an overlay side panel for this element, so my main call-out is making sure the feed doesn't pop out of existence when the sidebar animation starts!

[shortwavesurfer2009]

I don't mind the change! It makes sense for the primary action to stay where it is. I guess the only downside is that there's no longer an explicit close button.

We will leave it in then, and just make sure that we can close it when we tap outside or swipe down (I think swiping down doesn't work fully at the moment)

On another note, I'm trying to get the community sidebars to work again, but it's been a bit more difficult. I did make some changes which allows it to work at the moment, but it looks different visually. I've expanded the width of the sidebar to take up the whole screen. Personally, I think I like it but I wanted to get some thoughts on this! If we don't like this change, I can continue to work on getting it to match the previous implementation (although it might require some hacky solution)

The main thing thats causing the issues is that I'm mixing both sliver widgets and renderbox widgets. This makes it harder to make the sidebar into an "overlay" where the posts show up behind it. Instead, only a plain background is shown (you can see it briefly when I close the sidebar)

Since this is a visual change, I'll pull in @CTalvio and @machinaeZER0 for their thoughts if they have any!

Not having an explicit "close" button could screw up closing with talkback

Seems like the majority here goes towards the close button so I'll make that change!

Edit: nevermind. That is what i get for not reading the whole thread before commenting.

[hjiangsu]

I'm just going to add these so that I have someplace to keep track of all the current issues with the refactor since my last commit 🫠:

• Refreshing doesn't show circular progress icon properly
• When not logged in, you cannot close the community sidebar without going back (the general case is that if swipe actions are not available, you cannot dismiss the sidebar
• You cannot tap to dismiss community sidebar
• Disable or show error when specific FAB actions are performed which require login (dismiss read)
• Re-implement ability for FAB to be dismissed
• Switching sort types does not scroll user back to top
• Switching accounts from drawer does not refresh feed - this is only from anonymous - anonymous accounts
• Disable FAB doesn’t do anything
• Add back subscribe action on communities
• Creating a new post FAB action is broken
• Community sidebar actions are not working (new post, subscribe, block)

[hjiangsu]

@micahmo, would you be able to do a run through of the app to see if there are any more major issues that should be addressed? I think I got most of them, and any of the remaining ones can be addressed in a future PR. I want to get this out if possible because this is already such a large PR that it's hard for me to keep track of all the changes 😅

I'm sure theres still a lot of cleaning up to do, and I'll try to get to those on an iterative basis. As this is such a big change, I want there to be enough time in the nightlies and pre-releases so that we can catch as many issues as possible before pushing this out to the general releases. I imagine there won't be a general release until we get 0.19.x compatibility regardless so the sooner the better 😄

A couple of things to note moving forward:

• I'm separating the concept of "feed" vs "community". The feed will abstract logic strictly related to a given feed (general, community, user, etc) whereas community will hold anything strictly related to a community. So far, the refactor only takes into account general and community. The user feed portion will be in a separate implementation/refactor as there's so much already in this PR
• The FeedPage is now the replacement for CommunityPage. Most of not all the community related widgets and files will be deprecated in favour of the FeedPage when it makes sense to. Right now, the structure of the codebase is a bit messy but I'll try to make it a priority to clean things up and move things to their respective areas
• I've renamed the previous community_bloc to community_bloc_old since there are some places which relies on those functions (namely user_page as it has not been transitioned yet)
• There are now some "global" blocs in thunder_page which acts as a way to trigger events "globally" so to speak. I don't think this is the best pattern and I do have some ideas on how to potentially improve this for future iterations.

There's probably a whole other bunch of changes that I forgot to list out, but I think those are the main ones!

[micahmo]

@hjiangsu Once again, thanks for all the awesome work on this!

I ran through everything we've discussed in this PR so far, and only two main things stand out as still not working. It's totally up to you whether these should be addressed now or later. I think this is otherwise pretty good to go!

1. I still don't see taglines. I am using programming.dev for reference.

   EDIT: Something funky is going on with taglines. If I log into my programming.dev account, then they work, even if I switch back to anonymous mode. But if I remove my account, switch to anonymous, and restart the app, they're gone again. Note that I can't reproduce this prior to the refactor.

qemu-system-x86_64_TEVh9g6uRq.mp4

2. The FAB action "Expand options" does not work as either single-press or long-press, so the only way to open it is via a swipe up (which is an accessibility issue).

qemu-system-x86_64_oySA0sofVI.mp4

I'm planning on driving this branch during the day tomorrow in case anything else pops up, but I'd give it the green light otherwise!

[micahmo]

Hey @hjiangsu just checking in again after driving this branch for the day. I noticed a couple more things that aren't working quite right, of different severities. Again, totally up to you whether they are addressed in this PR or elsewhere.

1. There is a gap between some of the icons in the header.

2. Image and link previews cannot be opened from the feed or post. They work fine from comments.

[hjiangsu]

Hmm, I think I can leave those for a separate PR and just merge this one in! We'll fix those inconsistencies and issues that you mentioned as this one's gotten way too large and out of hand 😅

[hjiangsu]

Image and link previews cannot be opened from the feed or post. They work fine from comments.

I wasn't able to reproduce this issue - I can tap on a link or image and it opens up the corresponding preview/browser. Could you provide a video of it? Thanks!

[micahmo]

I think I can leave those for a separate PR and just merge this one in!

Sounds good!

I wasn't able to reproduce this issue - I can tap on a link or image and it opens up the corresponding preview/browser.

Sure! It's reproducible for me on my emulator and physical device. Here's a recording after getting the latest develop.

qemu-system-x86_64_9i8l3SsooJ.mp4

Without really digging in, it looks like some bloc is missing in the hierarchy.

Could not find the correct Provider<UserBloc> above this LinkPreviewCard Widget

Let me know if you want a hand looking at this since it's kinda broken at the moment. 😆

[hjiangsu]

Oh I see - it relates to the thumbnail previews being on the right side 😅 I just tested it with it being on the left (and also marking media as read)