msfjarvis / compose-lobsters

Claw for Android: Unofficial read-only client for https://lobste.rs, built using Jetpack Compose.
https://play.google.com/store/apps/details?id=dev.msfjarvis.claw.android
MIT License
99 stars 8 forks source link

Add tablet specific UI #686

Closed ThanaReka closed 2 weeks ago

ThanaReka commented 3 weeks ago

Hi @msfjarvis! Please have a look at what I have been able to implement so far. When trying to call the ListDetail version in the MainActivity I realized that the MainActivity is based on the BaseActivity class, and I wasn't exactly sure how to set the parameters for ListDetail. Let me know your thoughts when you get a chance. Thanks!

msfjarvis commented 3 weeks ago

You don't need to keep updating this branch with the main one. If there are conflicts I'll resolve them before merging. Right now all the merges are just making a mess of your commit history.

ThanaReka commented 3 weeks ago

You don't need to keep updating this branch with the main one. If there are conflicts I'll resolve them before merging. Right now all the merges are just making a mess of your commit history.

okay, thanks for letting me know.

ThanaReka commented 3 weeks ago

I haven't had a chance to look at your comments on the previous commit to update my code accordingly, but I wanted to add the TwoPaneLayout that somehow got missed earlier when I started noticing the errors.

ThanaReka commented 3 weeks ago

The actual code for TwoPaneLayout is missing so I don't really know why you've made the changes you have, but most of the code in this PR can be discarded to just replicate what the existing code in LobstersPostsScreen does.

ThanaReka commented 3 weeks ago

The actual code for TwoPaneLayout is missing so I don't really know why you've made the changes you have, but most of the code in this PR can be discarded to just replicate what the existing code in LobstersPostsScreen does.

Hi @msfjarvis! not sure if you had a chance to see my code for the twopanelayout from the commit I made yesterday, but I feel like I am supposed to use the code you have for postActions instead of what I have on line 49 there except I'm not exactly sure how. Can you point me in the right direction with this? Thanks!

msfjarvis commented 3 weeks ago

The actual code for TwoPaneLayout is missing so I don't really know why you've made the changes you have, but most of the code in this PR can be discarded to just replicate what the existing code in LobstersPostsScreen does.

Hi @msfjarvis! not sure if you had a chance to see my code for the twopanelayout from the commit I made yesterday, but I feel like I am supposed to use the code you have for postActions instead of what I have on line 49 there except I'm not exactly sure how. Can you point me in the right direction with this? Thanks!

The parameters you are accepting can be removed and initialized like this https://github.com/msfjarvis/compose-lobsters/blob/baebc56ab1055adf031099483b466cccb4c6bbdc/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/screens/LobstersPostsScreen.kt#L103-L119

ThanaReka commented 3 weeks ago

Hi @msfjarvis! I am trying to wrap my head around how you are navigating from NetworkPosts to the CommentsPage in the following lines of code of your LobstersPostsScreen: https://github.com/msfjarvis/compose-lobsters/blob/baebc56ab1055adf031099483b466cccb4c6bbdc/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/screens/LobstersPostsScreen.kt#L261-L273 (so that I can do the same in my TwoPaneLayout). Can you clarify for me?

msfjarvis commented 3 weeks ago

Hi @msfjarvis! I am trying to wrap my head around how you are navigating from NetworkPosts to the CommentsPage in the following lines of code of your LobstersPostsScreen:

https://github.com/msfjarvis/compose-lobsters/blob/baebc56ab1055adf031099483b466cccb4c6bbdc/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/screens/LobstersPostsScreen.kt#L261-L273 (so that I can do the same in my TwoPaneLayout). Can you clarify for me?

https://github.com/msfjarvis/compose-lobsters/blob/f827c10990ca0e29eef28ba481fed5c8a86a30e9/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/ext.kt#L46

msfjarvis commented 2 weeks ago

I don't want to discourage you but this PR has been running for a little too long and having to review it multiple times is not a good use of the limited time I have for my side projects.

If you wish to get credit for your work here please commit the file for NetworkPostsForTwoPaneLayout so I can finish off this PR, otherwise I'll be reimplementing it myself in the next couple days.

ThanaReka commented 2 weeks ago

I don't want to discourage you but this PR has been running for a little too long and having to review it multiple times is not a good use of the limited time I have for my side projects.

If you wish to get credit for your work here please commit the file for NetworkPostsForTwoPaneLayout so I can finish off this PR, otherwise I'll be reimplementing it myself in the next couple days.

Sounds good. I implemented the changes that you suggested in my last commit, but that doesn't seem to resolve the issue that I mentioned in my previous commit. Anyhow, I agree that I have been working on this for too long, so I would just like to get credit for the work that I have done. Much thanks!

msfjarvis commented 2 weeks ago

I've refactored the code to address some UX gaps, here's a link in case you wanna check it out https://github.com/msfjarvis/compose-lobsters/commit/1fc8f9a2be9bf5ca0a37b8f5231ec5ae97ac4fbf

ThanaReka commented 2 weeks ago

Cool. Thank you for sharing!