Expandable second home section

[ueen]

Expand inbox and episodes sections to scrollable lists if enough space on home.

This is a minimal working version with a few kinks that would need ironing out, but i'll do that only if theres a prospect of merging this

(Maybe the home settings dialog would need to be fixed for this, because its not really clear that the suprise section would expand to episodes list, i think this is just a language/design issue, not a concetual one...or it might be - then there could be seperate inbox&episode expandable list sections that are disabled by default and only are enableable if theres only one other section selected)

closes #6229

[keunes]

Does this address #6229? If yes, can you state this in the description, so PR & feature request are properly linked?

would need ironing out, but i'll do that only if theres a prospect of merging this

I think that if you (strictly) implement the above feature request, there is a prospect of merging this. It has been discussed before and not been assigned the 'Needs: Discussion' label, so it should be safe.

its not really clear that the suprise section would expand to episodes list, i think this is just a language/design issue, not a concetual one

I agree it's not a conceptual issue. I'm not sure if we need to mention something in the 'Configure Home' dialog:

	Doesn't complicate dialog	Ensures predictable UX	Helps with discoverability
Always show explanation	❌	✔	✔
Show explanation when 2 (relevant) sections are activated	✔	✔	❌
Never show explanation	✔	❌	❌

I would give more weight to a clean UI than to discoverability. Considering the table above, I would go for displaying something when relevant.

[ueen]

Yes it adresses that, thanks, i want to spend the time i allocate for AP solving issues and coding and not too much searching and reading everything, although that would be very helpful, so thank you for finding the issue :)

Not sure what you mean with explanation but I feel like you missed on suggestion that would alays display these options but have them disabled (greyed out) when the dont make sense (two or more other sections enabled) and only would become available if one compatible section is checked. seems like something that would check all your boxes, no? :)

I will improve the settings dialog next, but the basic implementatin is already there (select only continue and suprise section in home settings dialog), look here
https://www.youtube.com/watch?v=NjojzTm6O7k&embeds_referring_euri=https%3A%2F%2Fforum.antennapod.org%2F&source_ve_path=Mjg2NjY&feature=emb_logo

Just needs some more polishing (filter message is cut off) and some testing, and i bet @ByteHamster has something to say about the code :)

[keunes]

i want to spend the time i allocate for AP solving issues and coding and not too much searching and reading everything

I understand, but please realise that what you don't do, comes down to us, limiting also the time we can spend on, for example, reviewing your PR(s) and engaging with discussions with other contributors (and there are some about whom I feel guilty for not following up on their proposals yet).

Anyway – thanks for adding it :-) Could I ask you to just change 'adresses' to a keyword understood by GitHub? That will automatically close the issue if/when the PR is merged, helping us keep the issue list clean and preparing the changelog for the next release.

Not sure what you mean with explanation

I was thinking of the following text in the Configure Home dialog, below the list of sections and above the text buttons: "With this selection, the second section will fill the full screen."

Although reading this now, it's a bit useless to have.

feel like you missed on suggestion that would alays display these options but have them disabled (greyed out) when the dont make sense

Not sure what you mean with 'always display these options' – can you explain/show? (I didn't see anything special in the video.)

Normally, if you follow the feature request, thisnPR shouldn't introduce any additional options/settings which may be disabled/greyed out.

select only continue and suprise section in home settings dialog

According to the feature request, this shouldn't be possible, because 'get surprised' is not a vertical list:

When only 2 sections are active, and the second section is a (regular) vertical list, change the second (bottom) section as follows

If you want to include the full episodes list, we need to discuss the UX on the forum first.

[ueen]

If you want to include the full episodes list, we need to discuss the UX on the forum first.

This is, and always was my main point. You know this, i was always very clear about this from the get go.
And because it doesnt make sense with suprise i would add dynamic options to the dialog as discussed above.

This mental inflexebility makes it really hard to get anything done, how long is my feature request standing 1 or 2 years? (your issue is younger but i proposed it way earlier). And to be honest i'm getting annoyed again contributing for AP.

[antennapod-bot]

This pull request has been mentioned on AntennaPod Forum. There might be relevant details there:

https://forum.antennapod.org/t/home-feature-expanable-episodes-list-and-inbox/3891/1

[ueen]

current state, like discussed (see forum)

@ByteHamster currently the expandable section logic is in HomeFragment, should i refactor into HomeSections? might be cleaner. anything else to keep in mind for the cleanup?

[ueen]

did some cleanup - > READY TO REVIEW

minor remaining issue: the filter/queue info from the toolbar is not displayed correctly, bc the fragment toolbar of episodes, inbox, queue is hidden but there is no alternative textview, like in the feedlist. I actually prefered the way it was/is displayed in feedlist (because it was more visible/noticable) and would advocate to bring it back - which would also solve this issue :)

[ByteHamster]

I'm a bit confused by this code. Can't we just show QueueFragmentInHome directly by HomeFragment, without having to use a dummy section?

[ueen]

Yes, but i feel its cleaner like this, so we have the TAGs and the isExpandable which makes it easier for settings and the save preferences. Plus the logic of expanding is mostly neatly in the section so home fragment just cares about displaying the section. You can look at the git history, I had it without the sections and there was some quirky logic in home fragment and not nice reflection.

[ByteHamster]

The expandable section is not really a normal home section and needs to be handled differently anyway. Having classes that do nothing or return null in most methods feels more like a hack than a direct implementation. Why can't you just make two arrays (one with tags and one with names) containing the new classes (EpisodesFragementInHome etc)? Then you can just add the corresponding Fragment with a similar switch statement as the home sections, just for the bottom half. No need to create dummy classes just to add the entries to the same switch statement

[ueen]

Hmm guess I can retractor again, but I'm traveling rn and will get to it in April earliest

[ueen]

sooo loked into it again, and would suggest du keep it like this, not using sections would mean the section title layout must be recreated which seems like a bad way of duplicate code and would make adjustements to the section layout more complicated.

[ByteHamster]

While it would duplicate the title of the home section, I think this is still the better option. The variant implemented right now adds both a RecyclerView and a fragment container to every single home section and then needs to deal with hiding the unused one. A lot of the code in the home section then need case distinctions for whether this is a normal home section or an expanded one. The expanded ones return null or dummy values in their methods, which causes quite a bit of "dead code" that makes things harder to understand for new contributors. Finally, these dummy sections are fragments that do basically nothing but showing a fragment themselves, which is a bit of unnecessary nesting.

If you think duplicating the title section is a problem, it could also be moved to its own XML file and included from the sections and expandable sections.

Bottom sections are a different thing using different layout, settings and setup code. Integrating this into normal home sections and then having case distinctions everywhere makes the code more complex than it needs to be.

[keunes]

Hmm guess I can retractor again, but I'm traveling rn and will get to it in April earliest

That's a pity, but travelling takes priority of course :-)

Shall we put it in draft status until then?

[ueen]

Please merge, using sections is the best way to ensure a consistent layout with titles over the bottom half

[keunes]

Removed the draft status based on @ueen's last comment. However, now that #7048 was merged first, this one needs further work to get rid of conflicts.

[ByteHamster]

This should be in the main strings file, together with the other strings (ui/i18n folder)

[ByteHamster]

Are you still interested in this PR, @ueen? I think this would be a nice feature but it still needs a bit of work and some fixing of merge conflicts