tylerbwong / stack

An Android app for browsing Stack Overflow and other Stack Exchange sites.
https://stack.tylerbwong.me
GNU General Public License v3.0
506 stars 34 forks source link

feat: add hot network questions widget #144

Closed G-Rath closed 11 months ago

G-Rath commented 1 year ago

Description

This implements the widget aspect of #142, which I realised is the main thing that is important to me - I'm happy to edit that issue to explicitly remove the ask for a view, but figure it's not a big deal as this still the meat of what's needed for that too.

~Overall this is pretty complete, there's just a couple of minor things I need to address - specifically:~

  1. ensuring the widget can recover from a bad state, especially as I think Android kills it for taking too long to load initially
    • ~I think this can partly be improved by ensuring the refresh onclick handler is registered asap, but also know this is because I'm probably incorrectly doing async calls i.e. I should be deferring the load of the hot questions to the background or something...~
  2. its possible to get the widget/app into a state where it doesn't open the question; I'm pretty sure this is related to create vs resume logic, because when it happens the app doesn't disappear the question details content
    • ~I think I know how to fix this as I spent an annoying amount of time initially figuring out how to get the app to open specific questions (namely I was having trouble with the intent extras getting passed around, until I discovered I had to give a unique request identifier and action) but have not had the time to try it out yet~
  3. ~misc. UI improvements, like having the refresh button actually show feedback when touched and ideally be grey (for some reason android:tint works but android:app doesn't...), and ideally it would be nice to have the question title text size itself according to content to always fit like the original widget (though that's not critical imo)~

I've tried my best to do things sensibly but this is my first time writing Koltin and my first time working on such a big Android app so I totally expect and am prepared to be told parts are wrong and that they should be redone - this is also why I've not written any tests.

I'm happy to keep working on this, but would appreciate help and guidance from some folks with more experience with Android and Koltin 😅 hence opening this as a draft until I've addressed some of the above.

Screenshot_2023-06-25-16-07-05-71_b783bf344239542886fee7b48fa4b892 Screenshot_2023-06-25-16-09-01-06_b783bf344239542886fee7b48fa4b892 Screenshot_2023-06-25-16-09-20-15_b783bf344239542886fee7b48fa4b892 Screenshot_2023-06-25-16-10-45-39_b783bf344239542886fee7b48fa4b892

Checklist

Resolves #142

G-Rath commented 1 year ago

I think I've gotten it as good as I can for now by myself - there is still a bit of an annoying initial loading UX race that goes on that isn't helped by how long it can take to load the list afresh, but I think it sits in the OS space (in particular, I think sometimes the widget is delayed because Android itself has delayed processing the intent) and I've not found anyway to handle that better.

Ultimately this boils down to users just having to sometimes press the refresh button a couple of times initially - this is actually consistent with the original widget, which is also why I think it's more of an Android OS thing.

Also I couldn't figure out the bug where the app would sometimes open on the old question and not switch to the new one - while I've had this happen frequently, I've not found a consistent way to reproduce it and it always can be fixed by just refreshing the widget to a new question so I don't think it should be worried about for now.

Finally, I ended up decompiling the APK for the original app to have a look at the layouts so the widget now resembles the original a lot more (not that it was too differently, but mainly the refresh button is now better and gives visual feedback)

G-Rath commented 1 year ago

@tylerbwong I've run the majority of CI locally and confirmed it passes since GHA won't run without approval :)

tylerbwong commented 1 year ago

@G-Rath This is awesome, thanks so much for your contribution! I'm unfortunately a little busy right now and haven't had much time to work on Stack lately, but I'll hopefully be able to get to this in the coming weeks. Again, thanks so much for your hard work!

tylerbwong commented 12 months ago
4x2 Widget Widget Selection
Screenshot_20230709_191502 Screenshot_20230709_191524

This is what it looks like for me when I run it on API 33. Is there a reason why I can't resize them?

G-Rath commented 12 months ago

This is what it looks like for me when I run it on API 33.

That's odd - would you mind playing around with a smaller minWidth setting? My guess is that it's because thats dp based (and probably why in Android 12 you can specify cells rather than dps but the min android version means we can't use that for now).

When I'm back on the laptop I use for doing Android, I'll look into what values the original app was using too.

G-Rath commented 12 months ago

@tylerbwong ready for another look - I've also adjusted the minWidth and minHeight of the widget to match the values used by the original widget; this might resolve your size problem, but if not could you play around with those values as I suspect that's what's doing that.

tylerbwong commented 11 months ago

@tylerbwong ready for another look - I've also adjusted the minWidth and minHeight of the widget to match the values used by the original widget; this might resolve your size problem, but if not could you play around with those values as I suspect that's what's doing that.

I think those values looks fine. Can we also look into specifying targetCellWidth and targetCellHeight for newer API levels? More info here.

Seems like there are other sizing attributes to use as well:

android:minResizeWidth="40dp"
android:minResizeHeight="40dp"
android:maxResizeWidth="120dp"
android:maxResizeHeight="120dp"
android:resizeMode="horizontal|vertical"
G-Rath commented 11 months ago

I didn't think that you could use them with the min android level, but maybe I'm wrong.

I'm on mobile right now so if you're comfortable would you mind just pushing up the min cell sizes if that's the last thing needed to land this? Otherwise I can try take a look over the next couple of days (or as a follow up if you're happy to merge as is for now)

I don't think it makes sense to allow resizing though because the widget is only really designed for one size - which makes sense because titles are generally less than three lines...

tylerbwong commented 11 months ago

I didn't think that you could use them with the min android level, but maybe I'm wrong.

I'm on mobile right now so if you're comfortable would you mind just pushing up the min cell sizes if that's the last thing needed to land this? Otherwise I can try take a look over the next couple of days (or as a follow up if you're happy to merge as is for now)

I don't think it makes sense to allow resizing though because the widget is only really designed for one size - which makes sense because titles are generally less than three lines...

I think they would just get ignored on newer API levels, but I agree that resizing probably doesn't make much sense. Never mind then!

G-Rath commented 11 months ago

No worries, thanks for landing it! How long does it usually take to get published to the play store?

tylerbwong commented 11 months ago

No worries, thanks for landing it! How long does it usually take to get published to the play store?

It's been quite awhile since I've published a new release to the Play Store. There was never a set cadence, just when there was something new/stable enough to release.

There have been a few changes since the last release so I have to do some testing, but I will try my best to land it within the next week or so.

G-Rath commented 11 months ago

Sweet as - let me know if you find anything that could help address, and I'll see what I can do